Caused by rSVEC3306124038e5: Introduce the vector-body class, which changed various typography-related styles to only apply to .vector-body (a new selector specific to Vector), rather than .mw-body-content (a selector used by all skins, and VisualEditor). .mw-body would also work.

There was a similar issue with Vector affecting the main VisualEditor editing area only a few months ago in May: T283014: Font and font-sizes broken in VE.

vector-body is not the same as mw-body-content. The former applies to all elements in the content area, which includes categories, and subtitles and the latter is a standardized class that should mean the same across all skins. I wouldn't advise mw-body as this is not standard at current time and might change in future.

I can advise what's the most future-proof thing to do here, and what to do to fix it, but I'm not too familiar with the 2017 wikitext editor and what is doing (I'm only aware of 2011). How do I enable it? Can you please add some replication steps?

It's a beta feature, https://en.wikipedia.org/wiki/Special:Preferences#mw-prefsection-betafeatures → "New wikitext mode". You can also reproduce using visual diff in visual editor.

@ppelberg I am probably not the best person to work on this fix. I have little experience with OOUI or this product I can certainly provide code review support and test this if necessary.

The fix should be in the wikitext editor code. The most future-proof approach here is to copy across classes from the parent item of the standard #mw-content-text to the new item. A better long-term solution would be to have some kind of core API for creating content area, but I don't have bandwidth to think about that too deeply right now.

$('.ve-ui-overlay-global .mw-body').addClass( document.getElementById('mw-content-text').parentNode.getAttribute('class') )

I don't know anything about this code so I'm not sure what the best way of doing this is.

In T287733#7325676, @Jdlrobson wrote:

@ppelberg I am probably not the best person to work on this fix. I have little experience with OOUI or this product I can certainly provide code review support and test this if necessary.

Understood, okay. This context is helpful to have...than you, @Jdlrobson.

The Editing Team will take responsibility for fixing this. Should we need help with code review and/or testing, I'll comment as much here.

Visual editor impacted
I've updated the task description to mention that the visual editor is impacted by this issue as well.

In T287733#7322551, @Jdlrobson wrote:

vector-body is not the same as mw-body-content. The former applies to all elements in the content area, which includes categories, and subtitles and the latter is a standardized class that should mean the same across all skins. I wouldn't advise mw-body as this is not standard at current time and might change in future.

I can advise what's the most future-proof thing to do here, and what to do to fix it, but I'm not too familiar with the 2017 wikitext editor and what is doing (I'm only aware of 2011). How do I enable it? Can you please add some replication steps?

I don't fully understand why you need the skin name in the class name? This seems like a strange architecture to me. We only load one skin at a time, and the class names should just describe their purpose. A skin prefix in the name make sense if the feature was unique to that skin, but here we are talking about paragraph spacing, which is universal. If styles specific to a skin need to be put in a shared CSS file (which itself is probably bad practice) then the body class skin-vector can be used for context.

I don't fully understand why you need the skin name in the class name? This seems like a strange architecture to me.

It's just a prefix. We started using the skin name as a prefix similar to how we do with extensions e.g mw-mf- and ve-. The main reason for this was there was a pattern of skins using mw- prefixed classes to give the impression that these classes were universal to all skins. This was confusing as many mw- prefixed classes are universal to all skins e.g. mw-indicator, mw-body-content. We've been trying to put the true mw- prefixed classes in the core skins files so they can be more relied upon.

Unfortunately for backwards compatibility, there are still a lot of mw- prefixed classes in circulation in skins that are not standardized.

In this case 'vector-body' is an unstable element unique to Vector, that can change at any time.

Hope that makes sense?

A skin prefix in the name make sense if the feature was unique to that skin, but here we are talking about paragraph spacing, which is universal.

If there is a need for a skin specific body class, that class should only be used to style the skin specific feature, e.g. one could have vector-sticky-header as that is a feature unique to that skin, but one should not use that class for style rules that are common across skins, so more specifically, I don't understand why we need .vector-body p rules, which are the cause of this bug.

I think we're getting confused here. There is no skin specific body class used here or needed. My understanding about this bug is that we have a rule that controls font-size on an element in the page that has the class vector-body:

.vector-body {
    font-size: 0.875em;
    font-size: calc(1em * 0.875);
}

This node has multiple children that need to inherit the font-size rule. We also have gadgets to consider:

VisualEditor tries to simulate this element and uses a class mw-body which it presumably assumed to be a div present in all skins, given the mw- prefix. However the mw-body class is not a universal class, so when Vector used vector-body instead it broke. We are working towards either removing mw- prefixed classes or making them universal when they are not universal to skin, but that's going to take a while.

The most generic solution I can suggest is:
https://phabricator.wikimedia.org/T287733#7325676

A skin specific fix would look like this:

if ( skin === 'vector' ) {
  $( '.mw-body' ).addClass( ' vector-body' )
}

So as far as I understand it, the only decision here is to choose one of the above solutions, and we can patch this. My understanding of the code is limited however so I would need to be given a hint about where to put this code.

It should be possible to apply content-area-like styles by adding a non-skin-specific classes (mw-body-content, mw-parser-output). Aside from VE's wikitext preview dialog, any other extension or gadget that wants to display content in a popup that isn't a child of the content area would need to use this feature. This informal API existed until the introduction of vector-body.

vector-body doesn't just set root font sizes, it also sets line heights and margins for headings, paragraphs etc. Those styles should be controlled by a skin-agnostic class like they used to be.

Copying the class list is not an acceptable solution either, as the main content area my have classes added to it in future (or in other skins) that wouldn't apply to a separate content area.

It seems like we are just repeating the discussion that was had on T279388. Timo and others repeatedly raised very valid concerns which were just dismissed as "out of scope".

I've suggested two fixes here, neither of which is controversial.

The first fix requires separate code for each skin so isn't workable.

The second has problems blindly copying all classes as I outlined in my previous comment.

Informal contracts have been a huge time suck in the desktop improvements project

I appreciate this is one of the major complexities of the project, but that doesn't mean they can just be removed, especially without a reasonable alternative.

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

[mediawiki/skins/Vector@master] Apply typography rules to mw-body-content elements outside vector-body

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

@Esanders would the above patch fix this for you? Happy to take on the technical debt if so. The use of the vector-body class is mostly to avoid breaking certain gadgets.

Thanks, replied on patch.

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

https://patchdemo.wmflabs.org/wikis/4320a60872/w/

Change 736861 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Apply typography rules to `.mw-body-content` elements outside `.vector-body`

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

I hate to bear the bad news, but this is even worse now after the patch, the text size has also shrunk.

Editor preview	Read mode

(testing on https://en.wikipedia.beta.wmflabs.org/wiki/Albert_Einstein)

I think this change has also caused a global change in font size.

Before this change (commit 2ba2af97cfc55240b25ba0abc5caba87447c2cc4):

After this change (commit da832cc53d2cb009a618182c6c7a85aea3e7fb6b):

Seen on beta and on my local.

I think this change has also caused a global change in font size.

What browser are you using @dom_walden ? Can you replicate this at https://en.wikipedia.beta.wmflabs.org/wiki/Main_Page?safemode=1 ?

I hate to bear the bad news, but this is even worse now after the patch, the text size has also shrunk.

@matmarex it looks like this rule is to blame:

.ve-ui-overlay-global {
    font-size: 0.875em;
}

which is what I badly communicated about - this needs Vector change AND a VisualEditor change.

If doing that is complicated, we can revert the Vector change to work out a better approach.

In T287733#7496368, @Jdlrobson wrote:

I think this change has also caused a global change in font size.

What browser are you using @dom_walden ? Can you replicate this at https://en.wikipedia.beta.wmflabs.org/wiki/Main_Page?safemode=1 ?

I can. Firefox 78.15.0esr from Debian bookworm.

In T287733#7496489, @Majavah wrote:

In T287733#7496368, @Jdlrobson wrote:

I think this change has also caused a global change in font size.

What browser are you using @dom_walden ? Can you replicate this at https://en.wikipedia.beta.wmflabs.org/wiki/Main_Page?safemode=1 ?

I can. Firefox 78.15.0esr from Debian bookworm.

I'm feeling a bit stupid now :).
Can you expand on what you are seeing with regards to font size being wrong? font-size appears to be 14px for me, which is what is in production.
Also is this the legacy Vector (https://en.wikipedia.beta.wmflabs.org/wiki/Main_Page?safemode=1&useskinversion=1) or the modern Vector (https://en.wikipedia.beta.wmflabs.org/wiki/Main_Page?safemode=1&useskinversion=2) ?

Do you know what CSS rule is causing the problem you are seeing? I'm not sure how https://gerrit.wikimedia.org/r/c/736861 could have impacted the global CSS so there's possible something I don't understand about how the not selector is behaving.

I'm sadly not at all sure what specific rule is causing the issue (or if it is even caused by this task), but the text seems to be much larger (and dev tools reveal that "computed" font-size is 16px compared to 14px in production) and the gap between lines seems to be much smaller

compared to a page on the production enwiki:

I can see why. We used a CSS selector .mw-body-content:not(.vector-body .mw-body-content), .vector-body, but this is not supported until Firefox 84 (according to https://caniuse.com/css-not-sel-list). In CSS3, :not() can only take a simple selector (https://www.w3.org/TR/selectors-3/#negation) – e.g. a single class selector. It only allows complex selectors like .vector-body .mw-body-content in the CSS4 spec.

I see, so presumably that not query is only acting on the vector-body class and has become truthy resulting in the rules applying twice?
e.g. `.mw-body-content:not(.vector-body .mw-body-content) is interpreted as .mw-body-content:not(.vector-body) TIL

I've reverted the patch. @majavah can you confirm that addresses the issue?

Seems like we dont want to use the not selector here and use the ve specific selector from patchset 1 so I'll prepare that again, but I also need that VisualEditor patch prepared too.

In T287733#7497279, @Jdlrobson wrote:

I see, so presumably that not query is only acting on the vector-body class and has become truthy resulting in the rules applying twice?
e.g. `.mw-body-content:not(.vector-body .mw-body-content) is interpreted as .mw-body-content:not(.vector-body) TIL

No, it causes the selector to be invalid, and all comma-separated rules to be ignored. https://www.w3.org/TR/selectors-3/#grouping "If just one of these selectors were invalid, the entire group of selectors would be invalid."

Can we use #mw-content-text as a temporary workaround?

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

[mediawiki/skins/Vector@master] Fix font size in editor preview

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

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

[mediawiki/extensions/VisualEditor@master] Adjust font style/content hierarchy for Vector

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

Change 738442 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Fix specificity for mw-body-content in VE overlay

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

Change 738441 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Fix font size in editor preview

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

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

[VisualEditor/VisualEditor@master] Increase specificity of source mode paragraph margin rule

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

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

[mediawiki/skins/Vector@master] Revert \"Fix font size in editor preview\"

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

Change 738516 abandoned by Bartosz Dziewoński:

[VisualEditor/VisualEditor@master] Increase specificity of source mode paragraph margin rule

Reason:

Not needed right now because the original patch is being reverted, we can restore it when we know where we stand.

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

Reverted because this change caused T295712: Font-size in VE too small, which is also a worse problem.

It also caused a problem with paragraph margins in the NWE, which we were going to fix with https://gerrit.wikimedia.org/r/738516, but I'm not merging that for now given that we're reverting the other patch.

Change 738937 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Revert \"Fix font size in editor preview\"

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

Change 738516 restored by Esanders:

[VisualEditor/VisualEditor@master] Increase specificity of source mode paragraph margin rule

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

Change 738516 merged by jenkins-bot:

[VisualEditor/VisualEditor@master] Increase specificity of source mode paragraph margin rule

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

Change 748824 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (9df59571b)

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

Change 748824 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (9df59571b)

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

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

[mediawiki/extensions/VisualEditor@master] Add vector-body class to diff/previews

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

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

https://patchdemo.wmflabs.org/wikis/4320a60872/w/

Change 755052 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Add vector-body class to diff/previews

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

Visual Diff looks good

Change 753169 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Esanders):

[mediawiki/extensions/VisualEditor@master] Fix line height in templates used dialog

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

Change 753169 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Fix line height in templates used dialog

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