Looking at visual diff output differences, the presence of this class on <body> in Parsoid output is a source of major CSS-based diffs in navboxes. With a quick git grep in mediawiki, it looks like this is added to UI messages, some special pages and maybe some skin output. So, maybe Parsoid needs to look at where and when this class is needed rather than add it unconditionally on the body tag.
Description
Details
Subject | Repo | Branch | Lines +/- | |
---|---|---|---|---|
Pretty much revert f01a5bc | integration/visualdiff | master | +0 -16 |
Status | Subtype | Assigned | Task | ||
---|---|---|---|---|---|
Open | None | T264123 Ensure Parsoid's outermost element matches expectations of CSS and skins | |||
Declined | None | T258719 Parsoid adds mw-content-ltr / mw-content-rtl class to <body> tag unconditionally whereas core parser doesn't seem to |
Event Timeline
.mw-content-ltr is added to #mw-content-text not body, but my understanding is that this class was to support older browsers (IE6) which didn't support attribute selectors. I think its redundant now and we should probably move away from it in favor of .mw-content-text [dir="ltr"] and .mw-content-text[dir="rtl"].
@Mooeypoo does this sense or are there good reasons to keep it?
I'm going to reopen this as a child of T264123, instead of closing as a dup, since I think this is a more actionable task than the parent.
There's an interface in Parser.php to specify the wrapper and the wrapper class, but it's fairly new (past 2-3 yrs) so it wouldn't surprise me at all to find that it is being misused in various special pages and UX elements etc. They probably need to be hunted down and the differences fixed one by one.
I'd like to add a moment of thought to adding .mw-content-text to the class collection.
We're currently featuring on Vector skin:
- div.content-container
- main#content.mw-body
- Several direct child elements div.mw-body-content, among those div#bodyContent.mw-body-content
- div#mw-content-text.mw-content-ltr|-rtl
- Several direct child elements div.mw-body-content, among those div#bodyContent.mw-body-content
- main#content.mw-body
I'm not opposing a change here, but we should, if we touch this, not fortify this naming mess by pursuing with adding a messy class name. if possible.
Further explored in T264173: A better CSS content class naming structure (for Vector)
Possibly related (even though task description is slightly confusing right now T150843: Add CSS class or DOM id for page content
Following up on T258719#6346946
The body tag will have either a rtl or ltr class depending on the /UI/ language
A div element with ID #mw-content-text wraps the parser generated content and has the mw-content- prefixed class
For example on
https://he.wikipedia.org/wiki/%D7%A8%D7%99%D7%99%D7%9B%D7%9C_-_%D7%A4%D7%A1%D7%A0%D7%AA%D7%A8_-_%D7%A9%D7%99%D7%A8%D7%99%D7%9D_(%D7%A1%D7%99%D7%91%D7%95%D7%91_%D7%94%D7%95%D7%A4%D7%A2%D7%95%D7%AA)?uselang=en&safemode=1 while the content is in Hebrew, the site UI is in English. The body tag communicates the UI language, the parser content element dictates the content language.
As far as I understand Parsoid has no notion of site UI, so should not add mw-content-ltr / mw-content-rtl class to <body> tag . It should add it to a div that's a child of the body if it wants to be consistent.
Thanks for the feedback @Jdlrobson. After reading the comments, and based on working with visual diffs, I see what the problem is. Parsoid's output is equivalent to content of <div class='mw-parser-output'>..</div> of the core parser, but we stuff that output in a <body> tag. As part of readviews, where Parsoid's output is now going to be part of some ParserOutput equivalent and go through OutputPage class, this will all get fixed.
For visual-diffing, I anyway strip out all the chrome and pare down to div#bodyContent.
I am going to decline this ticket as a result since there isn't anything to do maybe beyond adding a <div class='mw-parser-output'>..</div> wrapper and completing the readview integration via ParserOutput & OutputPage.
Change 654310 had a related patch set uploaded (by Arlolra; owner: Arlolra):
[integration/visualdiff@master] Pretty much revert f01a5bc
Change 654310 merged by Subramanya Sastry:
[integration/visualdiff@master] Pretty much revert f01a5bc