Page MenuHomePhabricator

DiscussionTools preview is missing TemplateStyles styling
Closed, ResolvedPublicBUG REPORT

Description

Steps to replicate the issue (include links if applicable):

  • On English Wikipedia, edit a blank page (a sandbox of some flavor). Add __NEWSECTIONLINK__ to turn on DiscussionTools. Make a line with something like ABC ~~~~ so as to trigger the reply tool. Save.
  • Click the reply button.
  • Add something like {{tq|DEF}} ~~~~

What happens?:
The preview of the comment is missing the styles that should be present due to its TemplateStyles tag.

image.png (477×295 px, 16 KB)

The appropriate styles (post save):

image.png (441×90 px, 11 KB)

What should have happened instead?:
Styles should be present in the preview version.

Software version (on Special:Version page; skip for WMF-hosted wikis like Wikipedia):

Other information (browser name/version, screenshots, etc.):
Doesn't appear to be an issue when the template is already present on the page, as one might find by replying to the post-save state of the replication steps above. Meaning the styles are loaded in that scenario.

Wondering if it might be related to T428215: The CSS-subpages of templates and CSS in Styles tab of Index pages is broken in the Page NS given the timing, which would make it a Parsoid issue most likely. Tagging for Parsoid read views as such.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

The API response for action=discussiontoolspreview includes a <link rel="mw-deduplicated-inline-style" href="mw-data:TemplateStyles:r1248585704"> instead of the <style> tag that should appear in the template's first occurrence. For example: https://en.wikipedia.org/w/api.php?action=discussiontoolspreview&format=json&formatversion=2&uselang=en&type=topic&page=Wikipedia_talk%3ASandbox&wikitext={{tq|DEF}}&sectiontitle=&useskin=vector-2022

This does not happen for this roughly equivalent action=parse request, which has a <style data-mw-deduplicate="TemplateStyles:r1248585704">... as expected: https://en.wikipedia.org/w/api.php?action=parse&format=json&formatversion=2&uselang=en&type=topic&title=Wikipedia_talk%3ASandbox&text={{tq|DEF}}%20~~~~&prop=text%7Cmodules%7Cjsconfigvars&pst=1&disableeditsection=1&preview=1&useskin=vector-2022

(Neither response looks like it was generated by Parsoid for me.)

I kept reading the code to see what we might be doing differently, and I realized that the DiscussionTools preview parses the content twice (to figure out whether it needs to add an automatic signature): https://gerrit.wikimedia.org/g/mediawiki/extensions/DiscussionTools/+/1e3b0a4b8dd6d81788a3340c5016e9aff8ec5742/includes/ApiDiscussionToolsPreview.php#49

The same request but with explicit signature gives the correct result: https://en.wikipedia.org/w/api.php?action=discussiontoolspreview&format=json&formatversion=2&uselang=en&type=topic&page=Wikipedia_talk%3ASandbox&wikitext={{tq|DEF}}%20~~~~&sectiontitle=&useskin=vector-2022

It looks like some state is improperly reused between parses in the DeduplicateStyles or DeduplicateStylesDOM pass.

Wondering if it might be related to T428215: The CSS-subpages of templates and CSS in Styles tab of Index pages is broken in the Page NS given the timing

This comment there: T428215#11991748 suggests that this may indeed be the same problem.

Ah, great - this largely confirms the hypothesis I've put on the other ticket - the pass used to be idempotent, and now it's not anymore :(

Change #1298735 had a related patch set uploaded (by Isabelle Hurbain-Palatin; author: Isabelle Hurbain-Palatin):

[mediawiki/core@master] Reset DeduplicateStyles state between different pipeline executions

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

Change #1298735 merged by jenkins-bot:

[mediawiki/core@master] Reset DeduplicateStyles state between different pipeline executions

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

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

[mediawiki/core@master] DeduplicateStyles: Don't keep state in global pipeline stages

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

I kept reading the code to see what we might be doing differently, and I realized that the DiscussionTools preview parses the content twice (to figure out whether it needs to add an automatic signature): https://gerrit.wikimedia.org/g/mediawiki/extensions/DiscussionTools/+/1e3b0a4b8dd6d81788a3340c5016e9aff8ec5742/includes/ApiDiscussionToolsPreview.php#49

The same request but with explicit signature gives the correct result: https://en.wikipedia.org/w/api.php?action=discussiontoolspreview&format=json&formatversion=2&uselang=en&type=topic&page=Wikipedia_talk%3ASandbox&wikitext={{tq|DEF}}%20~~~~&sectiontitle=&useskin=vector-2022

It looks like some state is improperly reused between parses in the DeduplicateStyles or DeduplicateStylesDOM pass.

We should create a separate task for the double-parse, since that seems like a performance problem.

Change #1298925 had a related patch set uploaded (by C. Scott Ananian; author: Isabelle Hurbain-Palatin):

[mediawiki/core@wmf/1.47.0-wmf.5] Reset DeduplicateStyles state between different pipeline executions

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

I kept reading the code to see what we might be doing differently, and I realized that the DiscussionTools preview parses the content twice (to figure out whether it needs to add an automatic signature) …

We should create a separate task for the double-parse, since that seems like a performance problem.

It'd be difficult to correctly detect whether the comment is signed without parsing the wikitext. It's only parsing a short piece (the text of your previewed comment), so I think it's fine.

Change #1298925 merged by jenkins-bot:

[mediawiki/core@wmf/1.47.0-wmf.5] Reset DeduplicateStyles state between different pipeline executions

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

Mentioned in SAL (#wikimedia-operations) [2026-06-09T13:40:10Z] <cscott@deploy1003> Started scap sync-world: Backport for [[gerrit:1298929|Simplify fragment processing (T423700)]], [[gerrit:1298926|Move ::getFragmentsToTransform() to Content{Text,DOM}TransformStage]], [[gerrit:1298927|OutputTransform: Rename DeduplicateStyles and ExpandToAbsoluteUrls stages]], [[gerrit:1298925|Reset DeduplicateStyles state between different pipeline executions (T428336 T428215)]], [[gerrit:1299497|

Mentioned in SAL (#wikimedia-operations) [2026-06-09T13:42:11Z] <cscott@deploy1003> cscott: Backport for [[gerrit:1298929|Simplify fragment processing (T423700)]], [[gerrit:1298926|Move ::getFragmentsToTransform() to Content{Text,DOM}TransformStage]], [[gerrit:1298927|OutputTransform: Rename DeduplicateStyles and ExpandToAbsoluteUrls stages]], [[gerrit:1298925|Reset DeduplicateStyles state between different pipeline executions (T428336 T428215)]], [[gerrit:1299497|Store indicators

Mentioned in SAL (#wikimedia-operations) [2026-06-09T13:55:01Z] <cscott@deploy1003> Finished scap sync-world: Backport for [[gerrit:1298929|Simplify fragment processing (T423700)]], [[gerrit:1298926|Move ::getFragmentsToTransform() to Content{Text,DOM}TransformStage]], [[gerrit:1298927|OutputTransform: Rename DeduplicateStyles and ExpandToAbsoluteUrls stages]], [[gerrit:1298925|Reset DeduplicateStyles state between different pipeline executions (T428336 T428215)]], [[gerrit:1299497

The patch is now available on all wikis and, from what I can see, fixes the reported issue.

Change #1298921 merged by jenkins-bot:

[mediawiki/core@master] DeduplicateStyles: Don't keep state in global pipeline stages

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