Change Details

How content generated by users is cleared and floated is poorly defined, and not shared by all skins. To solve this, the ResourceLoaderSkinModule enables styles by default that adds a pseudo-element to the end of mw-parser-output that clears all floats on the page. However, this pseudo-element however should apply to the parent element based on the learnings in T279008. # Acceptance criteria [] The rule inside the `content-parser-output` feature applies to the parent element (#bodyContent) using a CSS selector to make overriding by skins and site styles easier. [] The `content-parser-output` feature has been renamed to reflect its new purpose and existing callers have been updated. [] SkinTemplate::wrapHTML is renamed to something more meaningful that reflects the new name and all callers in core are updated. wrapHTML method is kept for backwards compability. [] Drop the Vector overrides. e.g. revert https://gerrit.wikimedia.org/r/677038 and [[ https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/679842 | I47d16eb8186efa83e158713d852b443bce9aee1c ]] # Background Historically all Wikimedia-deployed skin have an element at the bottom of the page that clears any content before it. In CologneBlue and Modern this is the footer (`#footer, .mw-footer`). In Minerva and Monobook it was an element above the footer (`#page-secondary-actions` in Minerva and `.visualClear` in Monobook). In Vector, there was a clear: both on the `#bodyContent`. In all cases, all of these clears carried no documentation to why they existed, except a vague notion that "they were for user-generated content". In T277218 we learned that editors expect their floats cleared. When auditing the skins on skins.wmflabs.org/ I (@jdlrobson) found that many of these skins, did not clear floats, and those that did, did so with `visualClear` like elements which led to a desire to move this definition into core and simplify the lives of skin developers. Much of MediaWiki is more complicated than it needs to be, and constraining in this way makes the UI much more predictable to skin developers. # The misunderstanding The method OutputPage::addHTML allows the addition of any HTML and means that `.mw-parser-output` can be followed by any arbitrary element. In practice, however, the only time when this ever happens in practice is in category pages. File pages, are a special case -but the parser output is always embedded inside an element #shared-image-desc and none of its sibling elements can be floated by edits to the wikitext https://en.wikipedia.org/wiki/File:Amtrak_Cardinal.svg Some pages do not contain an mw-parser-output element, notably special pages, but work on the invalid assumption that the skin will always clear any floats inside them. This was the case with the RCFilters page. In these cases, I think it certainly justifies adding the responsibility on the special page itself, so @krinkle I respectfully disagree with you on that. I wouldn't use a component from a component library that floated itself and relied on other components for its layout. Components should work in isolation. I believe we should fix each of these so that they are future-proofed. Some pages do contain an mw-parser-output element but as a child element of something else (e.g. edit pages) so this is not a problem. # The long term solution It's clear by the regression on Commons Wikimedia that the definition is wrong. The clear should return to the parent element of `.mw-parser-output`, however, this has implications on architecture that I don't want to rush into. The parent element with id="mw-content-text" does not have a consistent class name. It's either `mw-content-ltr` or `mw-content-rtl`. We should introduce a class that is always present to identify this element. e.g. `mw-content`. The method that wraps this is called SkinTemplate::wrapHTML - in the process of doing this it would be good to name that something more meaningful and reflects this name e.g. SkinTemplate::createContentHTML. There are lots of things labelled content so I don't want to rush into naming here to make that matter worse. In ResourceLoaderSkinModule the feature that provides these styles is called `content-parser-output` and requires that all CSS styling inside it is scoped to `mw-parser-output` (see note [1]). We should rename this feature to something like `content-<new name>` and expand its responsibilities to content created outside the parser. e.g. anything that can be a child of that element created by the `wrapHTML` method. Again, I don't want to rush that. [1] https://gerrit.wikimedia.org/g/mediawiki/core/+/a87fa828745a7cde05751a91048623e72ea147b9/resources/src/mediawiki.skinning/content.parser-output.less#6

How content generated by users is cleared and floated is poorly defined, and not shared by all skins. To solve this, the ResourceLoaderSkinModule enables styles by default that adds a pseudo-element to the end of mw-parser-output that clears all floats on the page. However, this pseudo-element however should apply to the parent element based on the learnings in T279008. # Acceptance criteria [] The rule inside the `content-parser-output` feature applies to the parent element (#bodyContent) using a CSS selector to make overriding by skins and site styles easier. [] The `content-parser-output` feature has been renamed to reflect its new purpose and existing callers have been updated. [] SkinTemplate::wrapHTML is renamed to something more meaningful that reflects the new name and all callers in core are updated. wrapHTML method is kept for backwards compability. [] Drop the Vector overrides. e.g. revert https://gerrit.wikimedia.org/r/677038 and [[ https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/679842 | I47d16eb8186efa83e158713d852b443bce9aee1c ]] # Background Historically all Wikimedia-deployed skin have an element at the bottom of the page that clears any content before it. In CologneBlue and Modern this is the footer (`#footer, .mw-footer`). In Minerva and Monobook it was an element above the footer (`#page-secondary-actions` in Minerva and `.visualClear` in Monobook). In Vector, there was a clear: both on the `#bodyContent`. In all cases, all of these clears carried no documentation to why they existed, except a vague notion that "they were for user-generated content". In T277218 we learned that editors expect their floats cleared. When auditing the skins on skins.wmflabs.org/ I (@jdlrobson) found that many of these skins, did not clear floats, and those that did, did so with `visualClear` like elements which led to a desire to move this definition into core and simplify the lives of skin developers. Much of MediaWiki is more complicated than it needs to be, and constraining in this way makes the UI much more predictable to skin developers. # The misunderstanding The method OutputPage::addHTML allows the addition of any HTML and means that `.mw-parser-output` can be followed by any arbitrary element. In practice, however, the only time when this ever happens in practice is in category pages. File pages, are a special case -but the parser output is always embedded inside an element #shared-image-desc and none of its sibling elements can be floated by edits to the wikitext https://en.wikipedia.org/wiki/File:Amtrak_Cardinal.svg Some pages do not contain an mw-parser-output element, notably special pages, but work on the invalid assumption that the skin will always clear any floats inside them. This was the case with the RCFilters page. In these cases, I think it certainly justifies adding the responsibility on the special page itself, so @krinkle I respectfully disagree with you on that. I wouldn't use a component from a component library that floated itself and relied on other components for its layout. Components should work in isolation. I believe we should fix each of these so that they are future-proofed. Some pages do contain an mw-parser-output element but as a child element of something else (e.g. edit pages) so this is not a problem. # The long term solution It's clear by the regression on Commons Wikimedia that the definition is wrong. The clear should return to the parent element of `.mw-parser-output`, however, this has implications on architecture that I don't want to rush into. The parent element with id="mw-content-text" does not have a consistent class name. It's either `mw-content-ltr` or `mw-content-rtl`. We should introduce a class that is always present to identify this element. e.g. `mw-content`. The method that wraps this is called SkinTemplate::wrapHTML - in the process of doing this it would be good to name that something more meaningful and reflects this name e.g. SkinTemplate::createContentHTML. There are lots of things labelled content so I don't want to rush into naming here to make that matter worse. In ResourceLoaderSkinModule the feature that provides these styles is called `content-parser-output` and requires that all CSS styling inside it is scoped to `mw-parser-output` (see note [1]). We should rename this feature to something like `content-<new name>` and expand its responsibilities to content created outside the parser. e.g. anything that can be a child of that element created by the `wrapHTML` method. Again, I don't want to rush that. [1] https://gerrit.wikimedia.org/g/mediawiki/core/+/a87fa828745a7cde05751a91048623e72ea147b9/resources/src/mediawiki.skinning/content.parser-output.less#6 # QA For each skin, open your browser developer console and run `$('.mw-body-content').length === 1`. This should report true for all available skins. [] Timeless (https://www.mediawiki.org/w/index.php?title=MediaWiki&useskin=timeless) [] Vector classic (https://www.mediawiki.org/w/index.php?title=MediaWiki&useskin=vector&useskinversion=1) [] Vector (https://www.mediawiki.org/w/index.php?title=MediaWiki&useskin=vector&useskinversion=2) [] Monobook (https://www.mediawiki.org/w/index.php?title=MediaWiki&useskin=monobook ) [] Minerva (https://www.mediawiki.org/w/index.php?title=MediaWiki&useskin=minerva) [] Modern (https://www.mediawiki.org/w/index.php?title=MediaWiki&useskin=modern) [] CologneBlue (https://www.mediawiki.org/w/index.php?title=MediaWiki&useskin=cologneblue)

How content generated by users is cleared and floated is poorly defined, and not shared by all skins. To solve this, the ResourceLoaderSkinModule enables styles by default that adds a pseudo-element to the end of mw-parser-output that clears all floats on the page. However, this pseudo-element however should apply to the parent element based on the learnings in T279008. # Acceptance criteria [] The rule inside the `content-parser-output` feature applies to the parent element (#bodyContent) using a CSS selector to make overriding by skins and site styles easier. [] The `content-parser-output` feature has been renamed to reflect its new purpose and existing callers have been updated. [] SkinTemplate::wrapHTML is renamed to something more meaningful that reflects the new name and all callers in core are updated. wrapHTML method is kept for backwards compability. [] Drop the Vector overrides. e.g. revert https://gerrit.wikimedia.org/r/677038 and [[ https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/679842 | I47d16eb8186efa83e158713d852b443bce9aee1c ]] # Background Historically all Wikimedia-deployed skin have an element at the bottom of the page that clears any content before it. In CologneBlue and Modern this is the footer (`#footer, .mw-footer`). In Minerva and Monobook it was an element above the footer (`#page-secondary-actions` in Minerva and `.visualClear` in Monobook). In Vector, there was a clear: both on the `#bodyContent`. In all cases, all of these clears carried no documentation to why they existed, except a vague notion that "they were for user-generated content". In T277218 we learned that editors expect their floats cleared. When auditing the skins on skins.wmflabs.org/ I (@jdlrobson) found that many of these skins, did not clear floats, and those that did, did so with `visualClear` like elements which led to a desire to move this definition into core and simplify the lives of skin developers. Much of MediaWiki is more complicated than it needs to be, and constraining in this way makes the UI much more predictable to skin developers. # The misunderstanding The method OutputPage::addHTML allows the addition of any HTML and means that `.mw-parser-output` can be followed by any arbitrary element. In practice, however, the only time when this ever happens in practice is in category pages. File pages, are a special case -but the parser output is always embedded inside an element #shared-image-desc and none of its sibling elements can be floated by edits to the wikitext https://en.wikipedia.org/wiki/File:Amtrak_Cardinal.svg Some pages do not contain an mw-parser-output element, notably special pages, but work on the invalid assumption that the skin will always clear any floats inside them. This was the case with the RCFilters page. In these cases, I think it certainly justifies adding the responsibility on the special page itself, so @krinkle I respectfully disagree with you on that. I wouldn't use a component from a component library that floated itself and relied on other components for its layout. Components should work in isolation. I believe we should fix each of these so that they are future-proofed. Some pages do contain an mw-parser-output element but as a child element of something else (e.g. edit pages) so this is not a problem. # The long term solution It's clear by the regression on Commons Wikimedia that the definition is wrong. The clear should return to the parent element of `.mw-parser-output`, however, this has implications on architecture that I don't want to rush into. The parent element with id="mw-content-text" does not have a consistent class name. It's either `mw-content-ltr` or `mw-content-rtl`. We should introduce a class that is always present to identify this element. e.g. `mw-content`. The method that wraps this is called SkinTemplate::wrapHTML - in the process of doing this it would be good to name that something more meaningful and reflects this name e.g. SkinTemplate::createContentHTML. There are lots of things labelled content so I don't want to rush into naming here to make that matter worse. In ResourceLoaderSkinModule the feature that provides these styles is called `content-parser-output` and requires that all CSS styling inside it is scoped to `mw-parser-output` (see note [1]). We should rename this feature to something like `content-<new name>` and expand its responsibilities to content created outside the parser. e.g. anything that can be a child of that element created by the `wrapHTML` method. Again, I don't want to rush that. [1] https://gerrit.wikimedia.org/g/mediawiki/core/+/a87fa828745a7cde05751a91048623e72ea147b9/resources/src/mediawiki.skinning/content.parser-output.less#6 # QA For each skin, open your browser developer console and run `$('.mw-body-content').length === 1`. This should report true for all available skins. [] Timeless (https://www.mediawiki.org/w/index.php?title=MediaWiki&useskin=timeless) [] Vector classic (https://www.mediawiki.org/w/index.php?title=MediaWiki&useskin=vector&useskinversion=1) [] Vector (https://www.mediawiki.org/w/index.php?title=MediaWiki&useskin=vector&useskinversion=2) [] Monobook (https://www.mediawiki.org/w/index.php?title=MediaWiki&useskin=monobook ) [] Minerva (https://www.mediawiki.org/w/index.php?title=MediaWiki&useskin=minerva) [] Modern (https://www.mediawiki.org/w/index.php?title=MediaWiki&useskin=modern) [] CologneBlue (https://www.mediawiki.org/w/index.php?title=MediaWiki&useskin=cologneblue)