Skip to content

refactor(search): delete the form block nothing parses - #6456

Open
e107help[bot] wants to merge 1 commit into
e107help/6321from
e107help/6301-form-type
Open

refactor(search): delete the form block nothing parses#6456
e107help[bot] wants to merge 1 commit into
e107help/6321from
e107help/6301-form-type

Conversation

@e107help

@e107help e107help Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Why

$SEARCH_TEMPLATE['form']['type'] ships in both core search packs, live in the legacy one and wrapped in a comment in the other, and nothing has rendered either since February 2016, when 3279283b6c commented out the parseTemplate() call that would have drawn it. The assignment that would have filled its {SEARCH_TYPE_SEL} placeholder is commented out beside it, which leaves sc_search_type_sel() unreachable. Its other placeholder, {SEARCH_TYPE_DISPLAY}, was still computed on every request into a block nothing rendered.

That is point 5 of #6301, and half-present is the worst of the three states it could be in. A theme author reading either pack finds a block to style and never sees it, and the next person to touch the file has to work out which of the two commented-out lines was meant.

Deltik's answer was to delete it rather than revive it. The form already carries an advanced toggle without it: the "Toggle Advanced Mode" dropdown item at search.php#L169 points at the same search-advanced block, so bringing the old block back would put a second control for one thing in one form.

What Changed

Six sites the issue named:

And one it did not, found by tracing: search_front::$legacyForm maps SEARCH_TYPE onto the block at search.php#L67-L76, so a v1 theme assigning $SEARCH_TYPE had it synthesised into $form['type'], which nothing renders. That entry goes too. $legacyForm doubles as the v1-shape detector in formTemplate(), which now matches on seven keys rather than eight: the only pack read differently is one that assigns $SEARCH_TYPE and none of the other seven, which is not a search form.

Folded in: the commented-out {SEARCH_ADVANCED_BLOCK} parse at search.php#L1208-L1212 sits in the four lines below the one deleted above, commented out by the same 2016 commit, and sc_search_advanced() at search.php#L352-L361 has drawn that same search-advanced wrapper live ever since. It is the rest of one stretch of debris, and the argument for deleting the block condemns it in the same words. The commented-out $_GET sweep above it and the preg_replace() predecessors below it are debris of other lines, and they stay for a general sweep.

Also folded in: $js_adv at search.php#L1136-L1143 and search.php#L1154-L1156 is built and read nowhere, and has been since b19f9ffbc3 in January 2021. It sits between two of the deletions above and is all that would have been left of that stretch, so it goes with them.

LAN_SEARCH_29, LAN_SEARCH_30 and LAN_SEARCH_75 stay in the language files. A third-party template or plugin may still read them.

How It Was Tested

e107_tests/tests/unit/searchFormTemplateTest.php is new. It reads every shipped search pack with the tokenizer and asserts that each form block the pack declares is one search.php reads, with a second test holding the list of readable blocks down to what the file actually subscripts. It reads a pack's comments as source too, so a commented-out declaration counts as a declaration, which is what it is to the theme author reading the pack: wrapping the block in /* */ rather than deleting it reds the test as surely as leaving it live does. Reads of the template array in search.php are counted live only, because a commented-out read must not legitimise a block.

That is the guard against the block coming back, which a deletion on its own cannot express. Proven red by reverting the three source files and running it again.

The whole unit suite runs green on this branch: 2748 tests, 26085 assertions, 6 skipped. The downgrade tooling is at its fixed point on every changed file, so nothing here would be rewritten for the older cell CI runs.

Nothing rendered changes, which is why the test asserts on the packs rather than on markup. I traced every reader of each deleted symbol and a repo-wide search finds none left outside the language files; no bundled theme or plugin names any of them. The two tests that render /search.php and assert on its markup, searchAdvancedLabelsTest and searchAdvancedMultiTypeTest, stay green.

Backwards Compatibility

No rendered HTML changes for any theme, Bootstrap or legacy.

search_front::sc_search_type_sel() is public and goes away, so a third-party search template naming {SEARCH_TYPE_SEL} stops resolving it. On master that is the point of the change: the shortcode has had no reachable caller since 2016, and a template naming it has been rendering an empty string for a decade either way.

One narrow case, which I could not find an instance of: a third-party pack that moved {SEARCH_TYPE_DISPLAY} out of the type block and into message or end, the two blocks parsed with simpleParse() against $SEARCH_VARS, used to receive style='display: none' on a page without an advanced type and now receives an empty string, so an element it hid by default becomes visible. The placeholder only ever made sense on the row that never rendered, so putting it in a rendered block took noticing that first.

No twin on release/v2.3.x. The dead block is there too, but this is dead-code removal rather than a correctness fix, and dropping a shortcode a third-party template could name is a break the stable track should not take mid-series. Same shape as the front-end FAQ removal, which landed on master and left the stable branch's remains alone.

Stacked

This branches off #6450, which branches off #6379, because all three touch search.php. Merge order: #6379, then #6450, then this one.

AI Model

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

Checklist

  • One issue per PR: the diff is scoped to this change only
  • Commit messages explain why, not just what
  • New or changed behavior has test coverage (or explain why not)
  • No unrelated reformatting, renames, or import reordering

Refs #6301

$SEARCH_TEMPLATE['form']['type'] shipped in both core search packs, live
in the legacy one and wrapped in a comment in the other, and nothing has
rendered either since 3279283 commented out the parseTemplate() call
in February 2016. The assignment that would have filled its
{SEARCH_TYPE_SEL} placeholder was commented out in the same pass, which
left sc_search_type_sel() unreachable. Its other placeholder,
{SEARCH_TYPE_DISPLAY}, was still computed on every request into a block
nothing rendered.

Half-present is the worst of the three states it could be in: a theme
author reading either pack finds a block to style and never sees it, and
the next person to touch the file has to work out which of the two
commented-out lines was meant. The form already carries an advanced
toggle without it, the "Toggle Advanced Mode" dropdown item pointed at
the same search-advanced block, so reviving the block would put a second
control for the same thing in one form.

$legacyForm mapped $SEARCH_TYPE onto the block, so that entry goes too.
It doubles as the v1-shape detector, which now matches on seven keys
rather than eight: the only pack read differently is one that assigns
$SEARCH_TYPE and none of the other seven, which is not a search form.

The same 2016 commit left a commented-out {SEARCH_ADVANCED_BLOCK} parse
in the four lines below the one deleted here, and sc_search_advanced()
has drawn that same search-advanced wrapper live ever since. It is the
rest of one stretch of debris, and the paragraph above condemns it in
the same words, so it goes in the same pass.

$js_adv sat between two of the deletions, built from $search_info and
from the Google id and read by nothing since b19f9ff, long before
this branch. It is all that would have been left of that stretch, so it
goes with them.

LAN_SEARCH_29, LAN_SEARCH_30 and LAN_SEARCH_75 stay in the language
files: a third-party template or plugin may still read them.

The rendered page is unchanged for every theme, which is why the
regression test asserts on the packs rather than on the markup. It reads
each pack with the tokenizer and counts a commented-out declaration as a
declaration, because that is what the theme author reading the pack
sees, so wrapping the block in a comment rather than deleting it reds
the test as surely as leaving it live does.

Refs #6301
@e107help
e107help Bot force-pushed the e107help/6301-form-type branch from 15417de to 045ff09 Compare September 14, 2026 15:09
@e107help
e107help Bot changed the base branch from master to e107help/6321 September 14, 2026 15:10
@e107help

e107help Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

The base here now names what this branch actually sits on. It was opened against master while carrying five commits from #6379 and #6450 underneath its own one, so the diff read as the whole line of work; the base is now e107help/6321 (#6450) and the diff is the single commit this pull request is about.

The head has moved from 15417de7b6 to 045ff09d81, and the change is untouched: same patch id against the new base as against the old one.

This is the top of the chain, seventh and last. The order for the whole block, and what a stacked base does at merge time, is in #6325.

@Deltik
Deltik added this pull request to stack #6494 September 14, 2026 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant