fix(search): render the custom search result prefix

[e107help]

Why

A search handler's result prefix has three settings in admin: the handler's own prefix, none, or a text of your own. The third one has never rendered anything in v2.

The two halves disagree about who resolves the setting. The front end collapses the mode into the text before the renderer sees either, at search.php#L806:

$pre_title = ($this->search_info[$key]['pre_title'] == 2) ? $this->search_info[$key]['pre_title_alt'] : $this->search_info[$key]['pre_title'];

The renderer then tests that value against the numbers again, at e107_handlers/search_class.php#L382-L393. On PHP 8 a piece of prose equals none of 0, 1 or 2, so no branch runs, $pre_title_output is never assigned, and the next line reads an undefined variable: the prefix is dropped and a warning is raised per result. On PHP 7 the same string was loosely equal to 0, so the prefix was dropped in silence instead.

The mode-2 arm was $pre_title_output = $pre_title;, which would have printed the literal 2 even if the mode had reached it intact. Both sides were wrong, which is why no PHP version ever rendered this.

Fixes #6327

What Changed

search.php forwards the two prefs as they are stored, the mode in $pre_title and the text in $pre_title_alt, instead of collapsing one into the other.

e107_handlers/search_class.php reads both, and its branch now ends in an else, so $pre_title_output is defined whatever the pref holds:

• mode 1, the handler's own prefix, unchanged
• mode 2, the site's text, trimmed and followed by a single space
• anything else, no prefix

The space is the same convention the addons already follow in their own prefixes (category_name." | ", LAN_SEARCH_71.": "). Without it a prefix of News would render NewsMy article, which is the fault #6298 was about, so shipping the feature without a separator would have been shipping it broken in a new way.

Whether this setting deserves repairing at all is a fair question, since nobody can have been using it: the radio and its text box at e107_admin/search.php#L356-L359 could go instead, along with the pre_title_alt pref. There is no recorded decision either way, and that choice is Deltik's, not this branch's. Repairing it leaves the option open; deleting the control would not.

How It Was Tested

e107_tests/tests/unit/e_searchTest.php gains four cases, in the file that already covers e_search rather than in a second one. Each seeds one categorised news item, renders the search page for it in a subprocess, and asserts the exact contents of the result's <h4> link:

• mode 2 with a text set, the case this fixes, which also asserts that no undefined $pre_title_output is reported anywhere on the page
• mode 2 with the text empty, which must not leave a stray space
• mode 1, the handler's own prefix and its separator
• mode 0, the title alone

The first case is red on the unfixed sources and green with them restored, confirmed by reverting both files against the committed test rather than by reading. The whole unit suite is green: 2705 tests, 25754 assertions, 6 skipped. The tree parses, and the downgrade tooling converges on it with no rewrites.

The news item is seeded inside the subprocess, because the suite's own transaction is invisible to another connection and the sample dump seeds no news at all, and it is taken out again from the test's _after() through a second booted CLI child, because a subprocess killed on its timeout runs no shutdown function. Word boundaries are switched off for the query so that the one regular expression path runs the same on MySQL 5.7 and 8.

Backwards Compatibility

Modes 0 and 1 render byte for byte as before, which the two guard cases above pin.

Sites on mode 2 start seeing a prefix that has never appeared, and stop collecting one undefined-variable warning per result. That is the point of the change, but it is a visible difference on upgrade for anyone who set that radio years ago and forgot.

No tag, attribute or class in the rendered HTML changes, so a legacy skin is unaffected.

A third-party legacy sfile handler that reads the global $pre_title sees the mode where a mode-2 handler used to hand it the text. Core ships no handler that reads it: $pre_title is declared global in exactly two places, and both are in this diff. $pre_title_alt is new in that scope and shadows nothing.

AI Model

Claude Opus 5 (claude-opus-5), as e107help.

Checklist

• Traced on master before any edit, and the line numbers re-checked against the branch point
• Reproduction test that fails without the fix, proven by reverting the sources
• Unit suite green
• Modes 0 and 1 pinned by their own cases, so the backwards-compatible half is not taken on trust
• Adversarial review, twice; the second pass covers the answers to the first
• No comments added outside one-line docblocks
• British spelling in the commit message
• Master only: release/v2.3.x carries the identical defect, and a twin there is Deltik's call rather than this branch's

[e107help]

Back in step with master: this branch is rebased onto 1f08ff8505 and its head is now c2e4a592d1.

Only e107_tests/tests/unit/e_searchTest.php ever conflicted, and it did twice, because #6332 and #6329 both appended their cases to the same end of the class this branch appends to. Each resolution keeps every line from both sides, measured rather than asserted: the resolved file diffed against master's copy and against the branch's copy drops no line from either. Master's five crop cases and its bbcode-excerpt case stay, this branch's four prefix cases and their helpers stay, and in _after() the in-process restores run first with this branch's subprocess cleanup after them.

search.php and e107_handlers/search_class.php merged without help, and the change in them is hunk for hunk what it was at the pre-rebase head fa21769a3e: both prefs reach the renderer, and its branch ends in an else so $pre_title_output is defined whatever the pref holds.

On the rebased tree the unit suite is green (2705 tests, 25754 assertions, 6 skipped), the downgrade tooling converges with no rewrites, and the case for this fix is still red with the two source files reverted and green with them restored.

[e107help]

Both of the remaining search branches went onto one master tip tonight, in a single pass, so this one and #6325 can be merged back to back without either re-conflicting behind the other.

This branch is rebased onto 8ef6e790b4 and its head is now d337b395b2.

What conflicted

e107_tests/tests/unit/e_searchTest.php again, and nothing else. Two regions this time, because master has since given the class a globals snapshot: the property block, and everything from _after() onwards.

Master's $globalsBefore, $probeTable and $probeBuilt stay, and so do this branch's $newsSeeded and its five constants. In _after() the in-process restores run first, master's globals loop and its probe-table drop, with this branch's subprocess row cleanup after them. Measured rather than asserted: the resolved file diffed against master's copy drops no line at all, and diffed against this branch's own copy drops exactly one.

That one line is unset($GLOBALS['search_chars']);, and its deletion is deliberate. Master now snapshots the page-scope globals in _before() and puts them back in _after(), and search_chars is one of the names it covers. Left in place the unset would have run after the restore loop and wiped a value the loop had just put back, so the branch's own cleanup is not merely redundant now, it is wrong.

One fix absorbed while here

The same meeting of the two mechanisms left a gap the rebase created. This branch makes parsesearch() read a seventh page-scope global, pre_title_alt, and master's snapshot list named only six, under a docblock that promises to be exactly the set parsesearch() reads. Nothing leaks today, because the only in-process caller here pins pre_title to 0 and never reaches that branch, but the suite shuffles, so the first case that sets pre_title_alt in process would have leaked a prefix into whichever case ran next while the file looked like it had that covered. The list at e_searchTest.php#L95 now names it.

Why the tests moved within the file

Each of these branches conflicted with master, and they also conflicted with each other: this branch and #6325 each appended their block to the tail of the same class, so merging either one would have left the other dirty.

So the two blocks now have different anchors. This branch's four prefix cases and their helpers sit at the boundary between the highlighting group and testGetCommentHandlerPath(), and #6325 keeps the tail. It is a pure move: the line multiset and the method set are identical to the file before it, and the cases neither depend on declaration order nor on each other.

The result is measured on all three pairings, and every one merges clean:

merge	result
master 8ef6e790b4 + this branch d337b395b2	clean
master 8ef6e790b4 + #6325 536b40e4de	clean
this branch d337b395b2 + #6325 536b40e4de	clean

So either order works, and the second one in does not need another rebase.

On the rebased head

The whole unit suite is green on PHP 8.2 and MariaDB 10.11: 2722 tests, 25816 assertions, 6 environmental skips. The downgrade tooling converges on the tree with no rewrites. The case this fix exists for is still red with search.php and e107_handlers/search_class.php reverted and green with them restored, re-proven on this base rather than carried over from the last one, which matters because master has changed the highlighting and the word-boundary regex underneath since.

Two things left open

Neither is a regression this branch introduces, and both are named here rather than fixed because the fix would touch a file #6325 also edits, which would put the collision straight back:

• renderSearchPage() is a third private copy of "boot a page in a subprocess and hand back output and exit status", alongside the one in searchCommentHandlersTest.php. The copies have already drifted: the older one asserts a boot marker before believing the output and this one does not, so a child that dies before search.php is reached reports as a wrong prefix rather than as a failed boot. One protected helper on the shared unit base is the right home for it.
• The cleanup child's exit status is discarded, so a cleanup that fails leaves its row behind in silence.

Recommendation

Merge, together with #6325 and in either order. The behaviour is pinned by four cases, modes 0 and 1 are held byte for byte by two of them, and the suite is green on the tip this is based on. The merge itself is Deltik's call, as is whether the two items above are worth their own issue.

[e107help]

The release/v2.3.x twin for this is open as #6482, which closes out the stable search stack.

It is the same source change, hunk for hunk, and it carries these four cases with it. What differs is only what that branch forces: the news item is seeded through the database API v2.3.x has rather than the query builder it has not, the insert sets news_category.category_meta_description because it is NOT NULL with no default there, and runInBootedCli() is named from the trait it lives in on that branch rather than from the unit base.

One of the two things left open above is fixed on the twin instead of here. Its cleanup child reports whether it took the seeded news item back out, and the case fails when it did not, which matters because the unit suite loads its dump once per run and opens no per-test transaction: a row that survives its case is live on the site for every test after it. Doing the same on this branch would touch e107_tests/tests/unit/e_searchTest.php where #6325 also edits it, and that is the collision both branches were rebased out of twice, so it stays open here rather than becoming a third rebase. If it is worth doing at all, after #6325 lands is the cheap moment.

The order is what it was: this one merges first and the twin follows it, and both of those are Deltik's calls.

[e107help]

This one has moved onto the family's chain rather than sitting beside it. The base is now e107help/6421 (#6440) instead of master, and the head has moved from d337b395b2 to a1bcb20484.

The change itself is untouched: the diff against the new base has the same patch id it had before the rebase, and the tests in it are the same four cases.

Why the move, when this branch and #6325 were measured as merge-in-either-order only yesterday: two more branches have since appended to the tail of the same test class, and with three lines of work writing at one anchor, "either order" stops being something that can be promised for one pair without re-proving it for the whole set. One chain replaces all of that. The order for the whole block, and what a stacked base means at merge time, is in #6325; this one is fourth.

Its Fixes #6327 only fires if the base reads master by the time this merges, which is what deleting e107help/6421 on its way through does.