Page MenuHomePhabricator

Text incorrectly classified as part of a template because of TemplateStyles
Open, LowPublicBUG REPORT

Description

Chrome Version 94.0.4606.61 (Official Build) (x86_64)
MacOS 11.6 (20G165)

Go to https://en.wikipedia.org/w/index.php?title=User:RoySmith/drafts/Givans_Creek_Woods&oldid=1047477638

Click "Edit" on the top menu

Move down to the "Current day description" section and put the cursor over the last line. Anywhere you are in the line, it will show that you are editing the "Convert, Reflist" template (see attached screenshot)

Adding some whitespace before the Reflist template works around the problem (https://en.wikipedia.org/w/index.php?title=User:RoySmith/drafts/Givans_Creek_Woods&diff=1047479688&oldid=1047477638&diffmode=source)

Screen Shot 2021-09-30 at 10.11.23 PM.png (428×2 px, 171 KB)

Event Timeline

matmarex subscribed.

This happens because the HTML <style> element, generated by the wikitext <templatestyles ...> in the {{Reflist}} template, becomes a part of the previous paragraph. Thus, the HTML paragraph closing tag </p>, generated implicitly, is part of the template – and this forces the template's range to be extended to the whole paragraph, so that it covers a balanced HTML fragment.

You can see this in the Parsoid HTML, available here: https://en.wikipedia.org/api/rest_v1/page/html/User%3ARoySmith%2Fdrafts%2FGivans_Creek_Woods/1047477638

image.png (2×3 px, 829 KB)

And it's actually the same in the old parser's HTML:

image.png (2×3 px, 776 KB)

This is probably not fixable.

I've started a thread at https://en.wikipedia.org/wiki/Template_talk:Reflist to see if this can be fixed from the template side.

Since a style tag has no rendering impact, we might be able to fix this in Parsoid with a little bit of tweaking by migrating the style tag automatically into the <div>. But, slightly low priority for us. If the template can be tweaked, that is probably quicker.

Moving the styles inside the element can cause FOUCs, so I'm not really a fan of that option... and anyway, this affects any TemplateStyles template, not just reflist, so there should be a general solution to the problem...

Arlolra moved this task from Needs Triage to Feature requests on the Parsoid board.
Legoktm renamed this task from Text incorrectly classified as part of a template to Text incorrectly classified as part of a template because of TemplateStyles.Nov 11 2021, 3:41 AM

FWIW it's a balanced fragment, it just doesn't have a single root. So at least in theory it should be possible to teach VisualEditor to handle it, but it seems like way more work than worth it. Just move the <templatestyles> tag inside the tag generated by the template. If scoped styles ever make a comeback in web standards, we'd want such a structure anyway. And it doesn't cause FOUC.

Matma left a comment onwiki to which I've responded there, but I'll copy-paste here so as to centralize discussion.

My assertion (that it can) is based on my understanding that HTML parsers will go node to node in order down the tree in the order the file is read. If the style element targeting the element of interest comes after the element, that means the HTML parser needs to get to that element first. Meanwhile, in parallel systems (I understand Firefox to be and I expect Chrome to be also), the CSS parser will have gotten a job or some other indication to start styling the element of interest, possibly before it will have gotten to the style definition. Which causes reflow aka FOUC for short. This also assumes the jobs system employed by the browser doesn't take care of all elements at any given DOM level first and then proceed to jobs at lower DOM levels.

In practice it may not show up much, but FOUCs are basically predicated on some data race existing anyway (or total misstyling I guess). I prefer to guarantee the non-existence of FOUCs the best I can, so styles go before elements.

I know for a fact that bottom/late loading styles, such as MediaWiki:Mobile.css, causes FOUCs, so it may be a matter of distance from the element to the styles of interest. Maybe that's a different root cause, but I'm pretty sure it's not.

Regardless of this question with reflist, you will want to review T303378: MobileFrontend removal of elements takes <templatestyles> tags with it, where the only workaround I know is to move any identified templatestyles tags (or their strip marker forms I guess, I haven't started working on the problem) out of their containing elements. You can call that a hard blocker for those particular components having internal TemplateStyles, so, work still needs to happen on the WMF side.

In general, I would much rather VE/Parsoid not be dumb.

An additional note to the above is that new browsers may do these things fast enough not to matter, but WMF has an interest in slow browsers and connections too, where the likelihood of FOUCs increases anyway. I don't want to add to that.

If a compromise is needed in wikitext, the right one is actually <div><templatestyles></div><div class="reflist">..., but this is pretty ugly to me and may not extend to arbitrary other TemplateStylesd things never mind questions of style scoping. (The other meaningful thing to be done would be the elimination of {{reflist}}, but maybe that's a pipe dream. ;) But again, that's just a single template.)

Regarding Tgr's "we'd want such a structure anyway", T176272: Decide on what to recommend for table style usecase remains open (perhaps another point to consider regarding this task), and I intend to leave a comment there on the point Sooner or Later because I have some relevant Common.css styles that I intend to migrate to TemplateStyles.

In fact, I wouldn't have thought/been thinking about styles locations if Tgr hadn't left T176272#4479728 4 years ago.