⚓ T283014 Font and font-sizes broken in VE

Subject	Repo	Branch	Lines +/-
Partial Revert "Remove mw-body-content from HTML that is not the article body"	mediawiki/skins/Vector	master	+7 -6
Add mw-body-content to surface element	mediawiki/extensions/VisualEditor	master	+3 -1
Add mw-body-content to .ve-ui-surface	VisualEditor/VisualEditor	master	+1 -1

• Esanders created this task.May 17 2021, 2:38 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 17 2021, 2:38 PM

• Esanders triaged this task as Unbreak Now! priority.May 17 2021, 2:38 PM

Looks like this was caused by https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/682774

I would imagine such a significant change may have broken other things.

CC @Jdlrobson @Krinkle

Change 692074 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/skins/Vector@master] Revert "Remove mw-body-content from HTML that is not the article body"

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

gerritbot added a project: Patch-For-Review.May 17 2021, 2:48 PM

Krinkle updated the task description. (Show Details)May 17 2021, 2:56 PM

There was a series of changes related to that class in multiple skins, other commits might need reverting or fixing too: https://gerrit.wikimedia.org/r/q/message:mw-body-content

ppelberg subscribed.May 17 2021, 3:38 PM

Other places that could be affected by these changes:

ContentTranslation
Preview within 2017WTE
Reference previews inside VE reference context popups
Visual diffs (both within VE, and on the history page)

Looking...

Should this block the train tomorrow?

ovasileva subscribed.May 17 2021, 5:25 PM

In T283014#7093444, @Jdforrester-WMF wrote:

Should this block the train tomorrow?

I think so, yes [i]. Thank you for raising this option, James.

i. This issue upends, what I consider to be, an important "promise" of VE: people having a [close to] 1:1 mapping between reading and editing modes.

Jdforrester-WMF added a parent task: T281147: 1.37.0-wmf.6 deployment blockers.May 17 2021, 5:38 PM

Change 692358 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[VisualEditor/VisualEditor@master] Add mw-body-content to .ve-ui-surface

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

Test wiki created on Patch Demo by ESanders (WMF) using patch(es) linked to this task:

https://patchdemo.wmflabs.org/wikis/4555942430/w/

• Esanders updated the task description. (Show Details)May 17 2021, 5:56 PM

• Esanders updated the task description. (Show Details)

Change 692361 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/VisualEditor@master] Add mw-body-content to surface element

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

• Esanders updated the task description. (Show Details)May 17 2021, 6:11 PM

• Esanders updated the task description. (Show Details)

Test wiki on Patch Demo by ESanders (WMF) using patch(es) linked to this task was deleted:

~~https://patchdemo.wmflabs.org/wikis/4555942430/w/~~

Test wiki created on Patch Demo by ESanders (WMF) using patch(es) linked to this task:

https://patchdemo.wmflabs.org/wikis/38c4a7e1dd/w/

DiscussionTools reply widget is broken by the patch that adds mw-body-content to MW surfaces unconditionally, as the widget was already within the mw-body-content, so the font-size: 0.875em rule is applied twice.

Conceptually the mw-body-content class should be added by the target class (ve.init.mw.DesktopArticleTarget/MobileArticleTarget, or just ArticleTarget if it applies to both) as that class is responsible for the integration of the surface into the context of the page.

• Esanders updated the task description. (Show Details)May 17 2021, 6:36 PM

• Esanders added subscribers: • mewoph, kostajh.

• Esanders updated the task description. (Show Details)May 17 2021, 6:45 PM

• Esanders updated the task description. (Show Details)May 17 2021, 6:50 PM

• Esanders updated the task description. (Show Details)May 17 2021, 6:54 PM

Test wiki created on Patch Demo by Jdlrobson using patch(es) linked to this task:

https://patchdemo.wmflabs.org/wikis/b2467b1736/w/

Change 692358 abandoned by Esanders:

[VisualEditor/VisualEditor@master] Add mw-body-content to .ve-ui-surface

Reason:

See I4833d1ca9fda4fc0bd433760e47fe7010f00db05

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

• Esanders updated the task description. (Show Details)May 17 2021, 7:09 PM

Change 692361 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Add mw-body-content to surface element

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

Jdlrobson removed a parent task: T281147: 1.37.0-wmf.6 deployment blockers.May 17 2021, 7:28 PM

Just to summarize the problem: Various extensions were trying to mimic the HTML markup of the content which was previously inconsistent across skins (notably Minerva).

In the VisualEditor case, Previously #bodyContent had the class mw-body-content. Now it doesn't. VisualEditor creates a sibling element designed to mimic #bodyContent but because the class is missing Vector wasn't styling it correctly. The style has now been added.

Big kudos to @Esanders for identifying the issue and testing various other places which follow this pattern.

Note, going forward, if this is a common need of extensions it may be useful to have an mw.util.bodyContent utility function to provide a more safe way of doing this in future but I leave that conversation for another time.

We'll (web) keep this open to assess impact anywhere else.

Jdlrobson added a project: Web-Team-Backlog (Kanbanana-FY-2020-21).May 17 2021, 7:31 PM

Jdlrobson mentioned this in T281147: 1.37.0-wmf.6 deployment blockers.May 17 2021, 7:34 PM

Adding a note: the Growth experiment link suggestion tool is not currently deployed to any wikis, although it is expected to go out within the next week. Setting the priority as high to allow for quick QA and triage of any other issues potentially found related to this bug. If we find anything that does not affect the main namespace or the list in the task description we can consider this normal or high priority for the time being (vs UBN)

ReleaseTaggerBot added a project: MW-1.37-notes (1.37.0-wmf.6; 2021-05-18).May 17 2021, 8:00 PM

Jdlrobson moved this task from Needs Analysis to Ready for Signoff on the Web-Team-Backlog (Kanbanana-FY-2020-21) board.May 17 2021, 8:03 PM

Test wiki on Patch Demo by Jdlrobson using patch(es) linked to this task was deleted:

~~https://patchdemo.wmflabs.org/wikis/b2467b1736/w/~~

Test wiki on Patch Demo by ESanders (WMF) using patch(es) linked to this task was deleted:

~~https://patchdemo.wmflabs.org/wikis/38c4a7e1dd/w/~~

Change 692074 abandoned by Jdlrobson:

[mediawiki/skins/Vector@master] Partial Revert "Remove mw-body-content from HTML that is not the article body"

Reason:

fixed by other means

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

Krinkle mentioned this in T283206: Category list and siteSub text unusually large font size (Vector regression), absolutely positioned elements relative.May 19 2021, 10:41 PM