Skip to content

fix(search): summarise a custom page from its meta description - #6440

Open
e107help[bot] wants to merge 1 commit into
e107help/6326from
e107help/6421
Open

e107help[bot] wants to merge 1 commit into
e107help/6326from
e107help/6421

Conversation

@e107help

@e107help e107help Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Why

The custom pages search addon has always meant to summarise a result from the page's own meta description, falling back to the body:

$res['summary'] 	= (!empty($row['page_metadscr'])) ? $row['page_metadscr'] : $row['page_text'];

e107_plugins/page/e_search.php#L118

page_metadscr is a real column on the page table (e107_core/sql/core_sql.php#L423), but the addon's own config() never asks for it (e107_plugins/page/e_search.php#L72). A compile($row) is handed exactly the columns return_fields names, so that condition has been false on every row and every custom pages result has been summarised from raw page_text. empty() swallows the missing key, so nothing is logged and nothing looks broken.

This is the one #6326's census missed. The two reads it did find both set $res['image'], a key nothing renders, which is what made them easy to spot; this one hides inside an empty() and changes what a visitor reads.

Per #6421, the decision was to select the column rather than delete the read.

What Changed

One array element. p.page_metadscr joins return_fields beside p.page_text, and compile() is untouched: the line quoted above now gets the column it was always written for.

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 own config() selects, so the test exercises the query and the read together rather than handing compile() a column the query would never have delivered:

  • a page with a meta description is summarised from it, which fails with A page body before the change;
  • a page that left the field empty is still summarised from its body, which covers the other half of the ternary.

The existing testEverySearchAddonFixtureUsesOnlyReturnedColumns() now covers this column too, because the page fixture row carries it: that case is the structural guard that stops the defect returning, and it also fails before the change.

Two private helpers carry that restriction: searchAddonColumns(), which is the column-name loop the existing case already had, and compileQueriedRow(), which filters a fixture row down to those columns before compiling it.

The whole unit suite on PHP 8.5 and MariaDB 10.11: 2729 tests, 25885 assertions, 6 skipped, green. php -l and the Rector downgrade dry run are clean on all changed files.

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. That is the point of the change, and it is worth being explicit about one consequence: page_metadscr is not one of the addon's search_fields (e107_plugins/page/e_search.php#L73), so a page matched on its body will now show a summary with no keyword in it. Per #6332 such a summary is head-cropped with a trailing ellipsis rather than centred on a match, which is the right treatment for an author's own description. 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#L470) and goes out through the same toExcerptText(), crop and highlight path as page_text (e107_handlers/search_class.php#L367), 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

#6325#6418 → this. This branch is based on #6418, not on master, because #6418 edits both files this touches; a branch cut from master would have conflicted the moment it landed. GitHub retargets this to master when #6418 lands.

Fixes #6421 below is inert while the base is not the default branch. It starts working after the retarget, so the issue closes on merge rather than needing a manual close.

What this deliberately leaves alone

  • The !empty() guard on the read. A meta description of nothing but whitespace is truthy, so it now wins over the body and renders as a blank summary. Tightening it to a trim() test here alone would make the search addon disagree with the two other places core decides whether a page has a meta description: page.php tests the same column for truthiness before building the meta tag, and the related-pages addon uses it as a summary with no test at all. That is a decision about all three, not a line to slip into this one.
  • p.page_metakeys and p.page_fields. Both are weighted search_fields on this addon and neither is in return_fields, which is the same class of defect as this issue and it bites four addons. Filed as Four search addons weight columns their own query never selects #6439, because which way it goes changes what a visitor can find.

AI Model

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

Checklist

  • The change is covered by a test that fails without it
  • The unit suite passes
  • No rendered HTML changes
  • British spelling in prose and commit messages
  • Every citation pinned to a commit

Fixes #6421

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.
@e107help

e107help Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Carried forward with the rest of the master search family tonight. This branch is now based on e107help/6326 at 4562233024, and its head has moved from 340c4848df to a45c9d2da5.

Its own change is untouched: the diff against its base has the same patch id it had before the rebase.

The whole block is measured and ordered in #6325, and this one is third in that order, after #6418. The note in the body about Fixes #6421 still holds, with one correction: this repository does not delete a branch on merge, so the retarget that makes the keyword live happens only if the base branch is deleted or the base is changed by hand first.

@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.

2 participants