fix(search): summarise a custom page from its meta description - #6441
e107help[bot] wants to merge 1 commit into
Conversation
The custom pages search addon has always meant to summarise a result from the page's meta description when the author filled one in, falling back to the body otherwise. The read has been there the whole time, but page_metadscr was never in the addon's return_fields, so the column never reached compile() and empty() swallowed the missing key: every result was summarised from raw page_text and nothing looked broken. Selecting the column is what makes the existing read work. A page with a meta description now shows that description as its search summary, which is a visible change on every site that has filled the field in, and the author's own wording is what the ruling asks for. page_metadscr is not one of the addon's search_fields, so a page matched on its body will show a summary with no keyword in it, which the results page head-crops with a trailing ellipsis. The fixture row gains the column, so the structural guard that a fixture may carry no column its addon never selects now covers this one too.
5bc75b5 to
9385f68
Compare
|
Rebased as well, @Deltik, to follow #6419. Head is now No conflict on this one. The single commit replayed with identical content and only its hunk offsets moved, because the block it edits in Checks, on Eighth in the merge order for the stable search queue: #6462 (comment). It waits on its master half #6440, which sits on #6418 and #6325 in turn. |
Why
The release/v2.3.x twin of the master change for #6421.
The custom pages search addon summarises a result from the page's own meta description when there is one, falling back to the body:
e107_plugins/page/e_search.php#L114page_metadscris a real column on thepagetable (e107_core/sql/core_sql.php#L391), but the addon's ownconfig()never asks for it (e107_plugins/page/e_search.php#L68). Acompile($row)is handed exactly the columnsreturn_fieldsnames, so that condition is false on every row and every custom pages result is summarised from rawpage_text.empty()swallows the missing key, so nothing is logged and nothing looks broken.Per #6421, the decision was to select the column rather than delete the read, on both branches.
What Changed
One array element.
p.page_metadscrjoinsreturn_fieldsbesidep.page_text, andcompile()is untouched.A cherry-pick from the master branch was not possible:
e107_tests/tests/unit/e_searchTest.phpis a different, shorter file here, carrying three fixture rows rather than eight. The production line is identical to the master one; the test work was written for this file's shape.How It Was Tested
Two new cases in
e107_tests/tests/unit/e_searchTest.php, each compiling a fixture row restricted to the columns the addon's ownconfig()selects, so the test exercises the query and the read together rather than handingcompile()a column the query would never have delivered:A page bodybefore the change;The existing
testEverySearchAddonFixtureUsesOnlyReturnedColumns()now covers this column too, because thepagefixture row carries it. Two private helpers carry the restriction:searchAddonColumns(), which is the column-name loop that case already had, andcompileQueriedRow(), which filters a fixture row down to those columns before compiling it.The whole unit suite under PHP 5.6: 1540 tests, 19333 assertions, 5 skipped, green. The same suite on PHP 8.5: 1540 tests, 19335 assertions, 4 skipped, green. A clean archive of the whole tree parses under PHP 5.6.
Backwards Compatibility
No rendered HTML changes, so legacy themes are unaffected.
What does change is the text inside an existing search result, on any site whose authors filled the meta description in.
page_metadscris not one of the addon'ssearch_fields(e107_plugins/page/e_search.php#L69), so a page matched on its body will now show a summary with no keyword in it, cropped from the head. Pages with the field empty are unaffected.The value is a plain string on the way in (
'data'=>'str', capped at 155 characters by the admin form,e107_admin/cpage.php#L458) and goes out through the same crop and highlight path aspage_text(e107_handlers/search_class.php#L374), so nothing new reaches the results page unescaped.Selecting one more column from the table the query already reads adds no join and no extra query.
Merge order
This branch is based on #6419, not on
release/v2.3.x, because #6419 edits both files this touches. It is eighth in the order for the whole stable search queue, which is kept in one place: #6462 (comment)A stacked pull request does not retarget itself. GitHub only moves the base up when the base branch is deleted after its own merge, so either delete #6419's branch straight after merging it, or point this one at
release/v2.3.xby hand first.The master pull request for #6421 merges before this one, and the pair goes in back to back.
What this deliberately leaves alone
The same two things the master pull request leaves alone, for the same reasons: the
!empty()guard on the read, which lets a whitespace-only meta description render as a blank summary, and the weightedsearch_fieldsthis addon never selects, filed as #6439.AI Model
Claude Opus 5 (
claude-opus-5) as e107help.Checklist
Refs #6421