Page MenuHomePhabricator

Vector 2022: Absence of #contentSub breaks on-wiki gadgets and user scripts
Closed, ResolvedPublic3 Estimated Story PointsBUG REPORT

Description

The #contentSub element was emitted to the HTML code unconditionally until the recent software update (T311421's patch https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/816844). Many gadgets and user scripts relied on it to prepend some information before the page content. However, since this week release of MediaWiki, Vector 2022 is the only desktop skin that emits it only if the <div> is not empty.

On enwiki, the #contentSub element is referred to over 14 thousand times in the User: and MediaWiki: namespaces, leading to possibly many issues and scripts not working properly.

One of affected scripts is which is available as gadget on some wikis https://www.wikidata.org/wiki/User:Yair_rand/WikidataInfo.js

Acceptance criteria

  • We'll restore the contentSub element as a temporary workaround for the long term solution T316830.
  • We will not guarantee it will look the same. Onus is on gadgets to update styles as necessary.

QA steps

  • run the code `$('<div>hello world</div>').appendTo( '#contentSub' )` on the main page and any article page. Confirm you see the text hello world at the top of the page.
  • Confirm no regressions are flagged on pixel https://pixel.wmcloud.org/ relating to the article title area.

QA Results - Beta

ACStatusDetails
1T315639#8296492
2T315639#8296492

QA Results - Prod

ACStatusDetails
1T315639#8296489
2T315639#8296489

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

At a glance this looks like it's because of T311421's patch 816844, which made rendering contentSub and contentSub2 conditional on the skin having content to put in them.

On the other side, this is a good moment to talk about the correct place to show info from our sister projects, now that we know that they will be deleted from the sight of the non logged-in users (and the discussion is stalled, because no one wants to talk about the elephant on the room).

This is a GOOD PLACE to have them.

@Theklan: This task is about the removal of #contentSub. This task is unrelated to aspects like where to (not) list sister projects. Please stick to topics - thanks.

Is not unrelated. Is can be completely related. Look at what has the removal of #contentSub broken at the Catalan Wikipedia, for example.

However, since this week release of MediaWiki, Vector 2022 is the only desktop skin that emits it only if the <div> is not empty.

Not the only skin. Minerva has never outputted this element so presumably gadgets have never worked there.

On enwiki, the #contentSub element is referred to over 14 thousand times in the User: and MediaWiki: namespaces, leading to possibly many issues and scripts not working properly.

https://global-search.toolforge.org/?q=%5C%23contentSub&regex=1&namespaces=2%2C8&title=.*%5C.js

Impact doesn't seem quite so bad. I'm only seeing 1408 cases across all wikis for JS files (I don't think it's worth focusing on CSS here). On English Wikipedia, there are 196 scripts impacted. That's quite a low number in my experience. (That aside, there is obviously there's an understandable need here to add things to a subtitle element.)

As a long-term solution, I would suggest that we introduce an explicit API here mw.util.addSubtitle going forward. This would create official and clear contract boundaries for both skin developers and gadget developers. Relying on the existence of DOM elements has proven to be fragile in the development of Minerva and Vector. We are planning to prefix all IDS with mw- in future, so contentSub would likely become #mw-content-sub so we need to prepare to migrate this code regardless.

On fr-wp, there are 19 scripts impacted, including one very useful for patrolling/reverting. Should we fix it locally (how?), or will this bug be fixed globally?

On fr-wp, there are 19 scripts impacted, including one very useful for patrolling/reverting. Should we fix it locally (how?), or will this bug be fixed globally?

@Jules78120 On Polish sites I've fixed the gadgets by adding the following lines before a code that appends to #contentSub (possibly fixing other user scripts as well as a side effect):

if(!document.getElementById('contentSub'))
    $('<div id="contentSub"></div>').insertBefore('#mw-content-text');

It basically re-adds the missing element if it's absent. Exemplary diff here: https://pl.wikipedia.org/w/index.php?title=MediaWiki:Gadget-WikidataInfo.js&diff=prev&oldid=67862746&uselang=en

I'm afraid I have to correct my former post, as I've just realised that WikidataInfo now appears right below pagetitle. So it seems to work for me. Thx.

Please could the Devs make a statement as to the "correct" way to add new content at the top of a page?
Is there a definition anywhere of the IDs that define the main structural parts of a page that are valid for all skins and where some essence of contract exists to ensure that these IDs are future-proof?

Should we assume that the best approach is to ignore contentSub altogether and insert into the top of bodyContent ?

$("#bodyContent").prepend( $("<div>New info</div>") )

When will the bodyContent ID be renamed to mw-body-content and where can we track this?

I would say that T316095 is also related to this, because PAGEBANNER goes in content-sub, but I'm in doubts, because I can see it working at Wikivoyage.

JJMC89 renamed this task from V22: Absence of #contentSub breaks on-wiki gadgets and user scripts to Vector 2022: Absence of #contentSub breaks on-wiki gadgets and user scripts.Aug 24 2022, 6:21 PM
JJMC89 added a subscriber: IZapico-WMF.

....
Impact doesn't seem quite so bad. I'm only seeing 1408 cases across all wikis for JS files (I don't think it's worth focusing on CSS here).

i must say, this comment is puzzling at best, infuriating at worst.
borking 1400+ scripts across all projects is "not that bad"?
this shows very cavalier approach and rude disregard for the work done by tons of people in many various communities, all meant to enhance the functionality of mediawiki code.
can't think of good excuse not to simply re-introduce it. if the skin itself does not want to use it, so be it, but pulling the carpet from underneath the feet of 1400+ scripts is unjustifiable.
similar disregard for community and user input held back the adoption of VE on many projects for a very long time, and this comment hints that vector-2022 may suffer similar consequence, which was/is a shame - they are both really fine improvements.

Please could the Devs make a statement as to the "correct" way to add new content at the top of a page?
Is there a definition anywhere of the IDs that define the main structural parts of a page that are valid for all skins and where some essence of contract exists to ensure that these IDs are future-proof?

Should we assume that the best approach is to ignore contentSub altogether and insert into the top of bodyContent ?

$("#bodyContent").prepend( $("<div>New info</div>") )

not saying the best approach is to prepend to body content, but i _am_ saying you should not use $("#bodyContent") as in your example, and instead use mw.util.$content.
this variable is not skin-dependent, and should be impervious to future DOM changes.

peace

this shows very cavalier approach and rude disregard for the work done by tons of people in many various communities, all meant to enhance the functionality of mediawiki code.
can't think of good excuse not to simply re-introduce it. if the skin itself does not want to use it, so be it, but pulling the carpet from underneath the feet of 1400+ scripts is unjustifiable.

similar disregard for community and user input held back the adoption of VE on many projects for a very long time, and this comment hints that vector-2022 may suffer similar consequence, which was/is a shame - they are both really fine improvements.

I'm sorry you've got this impression, but I think if you looked at the history of vector-2022 from a development point of view, I think you'd find that it's quite the opposite.

We simply wouldn't be able to change anything if we had to retain all existing CSS selectors and IDs. I don't know if you do development yourself, but if you do I'd urge you to take a moment to think about how a constraint of not being able to change any ID would impede you with developing a MediaWiki interface.

As part of the development of Vector 2022 we are trying to build a better ecosystem for gadgets as to be honest, nobody has cared about it up until this point and just relied on the fact that skins never change, which is not a good strategy. This strategy may takes a bit longer and will lead to some disruption, but I believe it's the right strategy on the long term as it will result in more stable gadgets.

#contentSub is not present in the Minerva skin and never has been so these gadgets have always been broken. We are hoping to provide a better API as I mentioned in T315639#8175177 but getting that right will take a bit more time, but please be assured we will do it: T316830

When will the bodyContent ID be renamed to mw-body-content and where can we track this?

The bodyContent element is only output in certain skins. Right now mw.util.$content is the best element to target and can be considered stable since it is available in the mw.util library. Right now according to the core code:

If you need just the wikipage content (not any of the

  • extra elements output by the skin), use $( '#mw-content-text' )
  • instead. Or listen to mw.hook#wikipage_content which will
  • allow your code to re-run when the page changes (e.g. live preview
    • or re-render after ajax save).

A gadget/site script should feel free to restore the contentSub if necessary while we work on T316830 :

if(!document.getElementById('contentSub'))
    $('<div id="contentSub"></div>').insertBefore('#mw-content-text');

We simply wouldn't be able to change anything if we had to retain all existing CSS selectors and IDs. I don't know if you do development yourself, but if you do I'd urge you to take a moment to think
about how a constraint of not being able to change any ID would impede you with developing a MediaWiki interface.

this is a straw argument. nobody is asking to retain "all existing css and ids". we are talking about one single element (and yes, it's style too), which, as it turns out, is a dependency of some 1400 scripts across all projects, according to your count.
the leap from "we can't retained them all" to "we can't retain this single one" is simply argumentative. you can easily retain this one, without impeding any vector-2022 progress.
(and yes, i happen to be a developer myself, and even happen to be in the position of authoring more than one script which was borked).

A gadget/site script should feel free to restore the contentSub if necessary

the problem with site-script (i.e., "mediawiki:common.js") doing it "once and for all" is subtle: before restoring it, we should verify it's not there, i.e., this should be done when "ready", like $(function() { restore if needed});
so this restoration happens via callback, some time after site script is run, and after scripts and gadgets are loaded, even when a gadget declares "site" as a dependency.
presumably, these scripts/gadgets will also wait something on "ready" before appending/prepending to contentSub, so it's a crap shoot - which code will run first, the piece that adds contentsub, or the piece that depends on it.
doing so means those scripts and gadgets will not be 100% borked anymore, only 50% or 30% or 5%.
so the only "safe" thing will be to patch all those 1400+ scripts, like @Yair_rand did for WikidataInfo, and @Msz2001 described above.
this of course will never happen - not all of them are well maintained, some exist in projects very thin on maintainers, some completely empty, who copied a working gadget from another project years ago, and will never patch it.

#contentSub is not present in the Minerva skin and never has been so these gadgets have always been broken

most gadgets and scripts indeed do not work on mobile view, regardless of whether or not they need contentSub (and any editor choosing minerva as her desktop skin is knowingly relinquishing gadgets).
this is another straw argument - the whole issue is entirely focused on desktop users, and even more narrowly - people who had "vector" as the default skin (i.e., vast majority of desktop-using editors), and are deprived of tools they installed and used because of this change.

DOM changes occasionally break scripts which relied on things they should not rely on. this is understood, and the communities routinely maintain them. this is not a demand to never break anything, but breaking 1400+ scripts across all projects, when it's so simple, easy and painless to un-break them is a bit too much to swallow.

tl;dr: cannot think of one good reason not to resurrect this one element, other then sheer stubbornness.

peace.

A gadget/site script should feel free to restore the contentSub if necessary while we work on T316830 :

I'm not sure recommending this is the right approach -- it increases the odds that we're going to wind up with scripts/gadgets that cause content issues whenever the change to the contentSub ID actually happens. I'd rather just turn the conditional-rendering off again, so we can at least only have to deal with one major change.

(Though maybe it's too late now, really. 😅)

I'd rather just turn the conditional-rendering off again, so we can at least only have to deal with one major change.

The reason it was removed is that there is some styles that come from core that introduced a bunch of whitespace for anonymous users when the element was empty. We'd need to address those styles before restoring the element, so it's not as straightforward as a simple revert (it would just be exchanging one bug for another). We'll get to this, but right now other higher priorities with Vector 2022 are taking our attention.

If there are style issues in a specific situation, then the solution is to change the styles, not the DOM.

The figure quoted above is 1400+ scripts broken by this change.
Yes, some have been fixed, but the lack of a clean solution is leading to a diverse range of hacks.
This lack of co-ordination is increasing technical debt, when the purpose should always be to reduce it.

Pending a more considered change, a simple revert would be the quickest and simplest solution.

As I've mentioned reverting is not possible.

That said, I've spoken to some other engineers in the team and we're happy to compromise here temporarily.

  • Remove all styling associated with #contentSub element and move it to direct children of element e.g. #contentSub > *
  • Restore the unconditional rendering of the #contentSub element

This may result in some visual changes to #contentSub but it will at least make the site subtitle render without any changes to gadgets on the short term.

Hoping we'll get to this next week.

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

[mediawiki/skins/Vector@master] Restore unconditional rendering of siteSub with styling modifications

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

Change 834600 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Restore unconditional rendering of siteSub with styling modifications

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

Test Result - Beta

Status: ✅ PASS
Environment: beta
OS: macOS Monterey
Browser: Chrome
Device: MBP
Emulated Device:NA

Test Artifact(s):

QA Steps

✅ AC1: run the code $('<div>hello world</div>').appendTo( '#contentSub' ) on the main page and any article page. Confirm you see the text hello world at the top of the page.

Screen Shot 2022-10-06 at 4.40.19 PM.png (1×1 px, 311 KB)

Screen Shot 2022-10-06 at 4.40.52 PM.png (1×1 px, 404 KB)

✅ AC2: Confirm no regressions are flagged on pixel https://pixel.wmcloud.org/ relating to the article title area.
No errors relating to article title were flagged.

Edtadros subscribed.

Test Result - Prod

Status: ✅ PASS
Environment: testwiki
OS: macOS Monterey
Browser: Chrome
Device: MBP
Emulated Device:NA

Test Artifact(s):

QA Steps

✅ AC1: run the code $('<div>hello world</div>').appendTo( '#contentSub' ) on the main page and any article page. Confirm you see the text hello world at the top of the page.

Screen Shot 2022-10-06 at 4.42.49 PM.png (1×1 px, 268 KB)

Screen Shot 2022-10-06 at 4.42.05 PM.png (1×1 px, 310 KB)

⬜ AC2: Confirm no regressions are flagged on pixel https://pixel.wmcloud.org/ relating to the article title area.
Not testable in production.

Please confirm that this change is also valid for legacy Vector

This is a bug for Vector 2022, not legacy Vector. Legacy Vector is not affected by this bug.