Thanks for the spot, @ssastry. T363107 has a description of the issue and two belt-and-suspenders patches to fix it. It doesn't look like 1.43.0-wmf.2 has been branched yet (?) but I'll submit them both to backport.

In https://github.com/fedwiki/wiki-security-passportjs/pull/43 there was a slight issue with Extension:OAuth requiring the HTTP Authorization header, as opposed to passing an access token in the query string.

Worked without too much trouble for fedwiki ( https://github.com/fedwiki/wiki-security-passportjs/pull/43 ) although the fact that our resource/profile endpoint required an HTTP Authorization header (as opposed to an access token) was a brief stumbling-block.

Two orthogonal issues here:

1. Current Parsoid calls back to the legacy Parser to handle various types of content. That can cause there to be multiple $parser objects used on a given page. Most *present* issues with "linear parsing" are due to the fact that, even though the parse is technically in order, data being attached to the Parser object is lost because there are multiple parsers in play.
2. Incremental parsing can still be done in the presence of store-modify-write sorts of changes to shared parser state as long as the shared parser state can be fully serialized for use in a cache key (ie, only reuse this fragment if the incoming parser state is identical to what it was last time) and deserialized (ie, when reusing this fragment, reset the shared parser state to what it what it was *after* the fragment was originally parsed). (Or, simply bypass any caching or incremental parsing if read-modify-write is done.)

Does AWB need to support Extension:OAuth ? That would be one solution...

Some discussion of this along with pragmatic solutions at https://river.me/blog/lua-multi-instance-subtemplates/

After the backport, tested https://test.wikipedia.org/wiki/User:Cscott/LST?useparsoid=0&action=purge and https://test.wikipedia.org/wiki/User:Cscott/LST?useparsoid=1&action=purge when 1.43-wmf.1 was on testwikis and couldn't trigger the deprecation warning, so calling this fixed.

Ok, here's the status:

1. The root cause is that some extensions which use dynamic properties of $parser aren't included in (or aren't adequately tested by) the gated-extensions CI, so there's a risk of log spam about dynamic properties this week during the train.
2. As mentioned above, I'm traveling to the MediaWiki Users and Developers Conference tonight-Friday this week, so I won't necessarily be available to deal with these in a timely manner.
3. The "proper" fix to any logspam found involves (a) a new phab task to record the need to eventually remove the newly added property (these should be children of T343227), (b) adding the dynamic property as a real property of Parser, and (c) patching the extension itself to replace any unset($parser->dynamicProperty) with $parser->dynamicProperty = null since after the unset even a declared property will become "dynamic" again and start to trigger warnings. Also phan often has "undeclared property" suppressions that then start to fail once the property /actually exists/ and so to unbreak the extension's CI those need to be removed.
4. One option is the backport the 4 patches to 1.43-wmf.1 and then either suppress any other logspam found during the train (it is harmless) or else backport additional pairs of patches as in item 3 above: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1020282 and https://gerrit.wikimedia.org/r/c/mediawiki/extensions/LabeledSectionTransclusion/+/1020284 for LST; and https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1020249 and https://gerrit.wikimedia.org/r/c/mediawiki/extensions/ProofreadPage/+/1020250 for ProofreadPage.
5. The other (preferred) option is to backport https://gerrit.wikimedia.org/r/1020231 to 1.43.0-wmf.1 to temporarily disable the deprecation warnings for 1.43.0-wmf.1. The ProofreadPage/LST patches will ride the train on 1.43.0-wmf.2 along with re-enabled deprecation warnings -- and if any further Bad Extensions are found, I will be back from the conference and available to patch and backport to fix them (or we can cherry-pick the "temporarily disable deprecation warnings" patch and rinse and repeat.)

This seems to have broken the train-blocker task for this week, T361395, which gives a RuntimeException when you go there. The previous task T361394 seems unaffected though, and far future tasks like https://phabricator.wikimedia.org/T400000 give a 404 as expected.

Yeah, and if T114424 were to be resurrected, it could/should use ParserOutput::setExtensionData, since all the code paths described have just parsed the banner template and thus have the ParserOutput available. PageProps is for when you want to index content independent of the parse, not when you just want to add metadata to the parse.

Rather than page properties, which are stored and indexed in a separate database table, probably ParserOutput::setExtensionData() would be sufficient for this use case.

See also T343229, T343230, etc for context.

In the discussion at https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1019302 the following method names were proposed:

• setNumericPageProperty( string $propName, int|float|string $numericValue )
• setUnsortedPageProperty( string $propName, string $value )
• setSortedPageProperty( string $propName, string $value, int|float $sortKey )

This reappeared as related to T358588 and maybe we should re-triage this as maintenance work for Content-Transform-Team .

See notes in {T362221#9704263} for the implications of adding external links as page dependencies.

The root cause is the redirect resolution inside Parser:statelessFetchTemplate which follows (a limited number of) redirects in order to obtain the "real" template text. It adds each step in the redirect chain to the page dependencies as it goes. The issue is that when redirects point to interwiki links, it goes ahead and adds those external links to the dependencies as well, which eventually causes them to be sent to ParserOutput::addTemplate(). As discussed in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/LabeledSectionTransclusion/+/1004305:

At the moment the call to Parser::fetchTemplateAndTitle (directly or via Parser::getTemplateDom) would register the interwiki as template via ParserOutput::addTemplate and it gets stored in the linktarget table with ns=0 and title=Non-Wiki-Part. When this is a interwiki to a user page, the linktarget has ns=0 and title=User:Name and namespaceDupes.php wants to repair this (the way I have seen this - T355706).

It's specifically a local transclusion of a page with a #REDIRECT, it doesn't happen if you put the #REDIRECT directly in the action=parse&text=.... request, nor does it happen if the page you are transcluding includes a interwiki link but not as a #REDIRECT. So probably something about how #REDIRECT is handled as a magic word when it is *not* stripped by WikitextContentHandler::extractRedirectTargetAndText. Also happens if parsoid=1 in the API request, oddly (?) enough, since Parsoid by default (a) doesn't strip the #REDIRECT and (b) handles the #REDIRECT locally (generating a <link>) rather than calling the core magic word. Hmm.

Yeah, the requests are definitely coming from the GlobalUserPage extension: https://codesearch.wmcloud.org/search/?q=GlobalUserPageAPIUrl&files=&excludeFiles=&repos=

Hm, could this be part of https://www.mediawiki.org/wiki/Extension:GlobalUserPage ? Although it's weird that it's coming in via an API request.

I'm already worried about the token-gluing possible with:

background: var(--foo) var(--bar) var(--bat);

which is allowed by the current patch. I guess adding default values doesn't make this *worse* but the semantics of var are already very worrying from a security standpoint. As long as users can't define their own custom properties we're probably okaaay, but we're relying on Codex and Skin authors not to inadvertently define custom properties that can be abused by juxtaposing them in unusual and unexpected ways.

In T359001#9691256, @Jdlrobson wrote:

What Parsoid provides:

<section>
<h2></h2>
<p></p><p></p>
</section>

What MobileFrontend needs:

<section>
<h2></h2>
<div><p></p><p></p></div>
</section>

The reason it needs the DIV is that the H2 hide/shows the section content.

The idea of skin-specific custom properties arose, but I think that can be done without adding additional var() syntax like so:

background-color: #fa11bac;
background-color: var(--skin-specific-custom-property);

Why do we need fallback values? On project, the custom properties should always be defined by codex, etc, and I'd think we'd actually want to hard fail with an error if an unknown property is referenced, rather than fallback.

I don't think MFE is getting canonical Parsoid HTML -- it is getting the output of ParserOutput:getText() which is postprocessed.
In particular, it is using the OutputPageBeforeHTML hook to fetch the final "read view" HTML and then munging it.

As I understand @matmarex's https://www.mediawiki.org/wiki/Heading_HTML_changes there /will/ be a mw-heading1 added once the legacy parser adopts the new heading HTML, but there's not one from the legacy parser currently (and that's expected). Any CSS rule which targets <h1> should also target .mw-heading1 in the way shown by https://www.mediawiki.org/wiki/Heading_HTML_changes#Skins_and_sitewide_styles.

T361330 was a repeat of the same issue; Ia8fd49a6f9af18e32d47d1dcd052c5f33123f44b adds a deprecation to ParserOutput to try to catch these (first with a deprecation warning, later with a thrown exception) in a more systematic way.

Thanks!

Thanks!

Seems like this is most likely an editing team bug (adding them), but we'll keep this on 'tracking' CTT to keep an eye on things in case it turns out to be a parsing issue.

Created https://gerrit.wikimedia.org/r/c/css-sanitizer/+/1016816 to add a HISTORY.md file for css-sanitizer.

Reviewed https://gerrit.wikimedia.org/r/c/css-sanitizer/+/1014653 ; once that is updated and merges I can help release a new version of css-santizier -- that library really needs a HISTORY.md file to document its release history. The dependency in TemplateStyles is ~5.0.0 so it should probably get its composer.json updated to something like "wikimedia/css-sanitizer": "^5.1.0 || ^5.2.0" in preparation for a 5.2.0 release of css-sanitizer.

The "more modern" way is indeed plugging into the OutputTransform pipeline in core, which would (eventually) allow caching the mobile-transformed content as well, so that would be a performance win. Right now the only extension point is the ParserOutputPostCacheTransform hook, which is a fine way to get started, but as you mention some of this code might want to be integrated in core with a new parser option to control it, something like ParserOption::setLazyLoadTransform(true) and then the transformation code would live in a new OutputTransform\Stages\ApplyLazyLoadTransform class in core. If the mobile transformations contain to live outside core, then we'll probably need to add a way for extensions to define their own output flavors, but those hooks aren't quite defined yet.

Note that I'm not 100% convinced we should do this -- see the caveats in T361413: Parsoid should perhaps use LinkUpdate job for lints instead of special Linter API/Hook.

We should probably add the lint errors to the ParserOutput object when useParsoid is set; after that is done it ought to be straightforward to export them from the parse API.