Page MenuHomePhabricator

Store page indicators as a set; select one at output time
Open, Needs TriagePublic

Description

Page indicators are currently "last write wins". We should fix this dependency on page order and instead collect all page indicators in a set, and select one when the ParserOutput is transferred to the OutputPage (not before).

This might involve some changes to the current "last write wins" semantics; "arbitrary selection" is probably easiest but non-reproducible (output may change every time an asynchronous fragment is evaluated). "lexicographically earliest" is probably simple to implement and reasonable in most cases; editors can probably tune the priority if needed by adding invisible characters.

Event Timeline

Change 759729 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/core@master] WIP: Collect all page indicators in ParserOutput, instead of \"last write first\"

https://gerrit.wikimedia.org/r/759729

Page indicators *are* currently stored as an associative array, but this is done under the HTML ID attribute for the page indicator as the key. This ID *is* supposed to be unique on the page (which is good for enforcing uniqueness w/r/t T300979: Ensure ParserOutput can always be combined asynchronously/out-of-order), but some extensions have to jump through hoops to get the order they want, cf:

	// @HACK: Add a tilde to force sorting towards the end of the indicator list,
		// because there's no way to set the indicators' order.
		// Its ID ends up as #mw-indicator-.7Eext-wikisource-download
		$out->setIndicators( [ '~ext-wikisource-download' => $button->toString() ] );

in https://gerrit.wikimedia.org/g/mediawiki/extensions/Wikisource/+/83f65d5f270a3064d529742bc694b511ee45552f/includes/HookHandler/ArticleViewHeaderHandler.php#61

On the other hand, that workaround is not too crazy, as these things go.

An improvement might be:

function setIndicator(string $id, string|DocumentFragment $html, int $priority=0);

You could also consider having the method be appendIndicator and explicitly allowing multiple indicators with the same ID. That has somewhat better composition semantics than enforcing uniqueness, in the (rare?) case where two different content fragments try to write to the same indicator ID.