There is a patch pending for this that uses similar approach to T42812: https://gerrit.wikimedia.org/r/#/c/372515/

Change 372515 had a related patch set uploaded (by Bartosz Dziewoński; owner: Fomafix):
[mediawiki/core@master] Hide TOC with CSS instead of JavaScript

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

Change 445258 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/core@master] Expanding/Collapsing table of contents should not cause reflow

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

Change 372515 merged by jenkins-bot:
[mediawiki/core@master] Hide TOC with CSS instead of JavaScript

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

Patch got merged from a volunteer so this should go through some QA.
Note, momentarily I saw two toggles:

Change 445300 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/core@master] Cached HTML/JS shouldn't show two toggle links

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

^ @Fomafix @matmarex not sure if this is necessary as cannot replicate but might want to merge this to be safe.

Didn’t look into a patch before, some small questions (not arguing against the patch or anything):

1. Does it mean that now you should just check the checkbox if you are modifying the TOC with scripts (Ctrl+F ‘TOC hidden’ in Russian Wikipedia’s Common.js)?
2. Is CSS-based solution entirely accessible? Here it says that IE11 doesn’t read CSS content.

In T195053#4417543, @stjn wrote:

2. Is CSS-based solution entirely accessible? Here it says that IE11 doesn’t read CSS content.

I don't have Jaws or NVDA, but with Windows Narrator, it indeed doesn't read it. I am not sure if it is worth the effort to support this browser.

We should probably add role="button" to the <input type="checkbox"> though!

Here's how Windows Narrator reads it:

(added Jul 16: Firefox gives the same results as Edge; Chrome doesn't seem to work with Narrator)

In T195053#4417543, @stjn wrote:

1. Does it mean that now you should just check the checkbox if you are modifying the TOC with scripts (Ctrl+F ‘TOC hidden’ in Russian Wikipedia’s Common.js)?

Yes. But you will need to keep the old code for a while and use it if the checkbox doesn't exist, because cached page HTML without it will continue to be used until it expires (in a week from the deployment? not sure).

Is CSS-based solution entirely accessible? Here it says that IE11 doesn’t read CSS content.

If we wanted to, I could see us addressing this by only using the CSS temporarily and disabling the pseudo selector of toctogglelabel and adding the text to the label in JS.

The problem from https://phabricator.wikimedia.org/T195053#4417392 is only present for 5 minutes until the cache for the style/script cache has expired and it only happens on new edited or purged pages which contain the new HTML.

To prevent this problem the toc.js change in https://gerrit.wikimedia.org/r/372515 can deployed in a prior release.

I think the double button problem is acceptable. It happens only for users, which make the following steps:

1. User X loads a page before deployment. -> Style/CSS cache contains the old toc.js. This cache expires after 5 minutes
2. Deployment
3. User X or any other user edits or purges the page Y.
4. User X loads the page Y. -> HTML contains the new code with a toggle button. The toc.js contains the old code from cache which also adds a toggle button. Two toggle buttons are visible.

Five minutes after step 1 the cache has expired and the problem can not occur anymore.

Unable to reproduce

In T195053#4419045, @Fomafix wrote:

I think the double button problem is acceptable. It happens only for users, which make the following steps:

1. User X loads a page before deployment. -> Style/CSS cache contains the old toc.js. This cache expires after 5 minutes
2. Deployment
3. User X or any other user edits or purges the page Y.
4. User X loads the page Y. -> HTML contains the new code with a toggle button. The toc.js contains the old code from cache which also adds a toggle button. Two toggle buttons are visible.

Five minutes after step 1 the cache has expired and the problem can not occur anymore.

Indeed. We support compatibility between the old HTML and the new CSS, but not the old CSS and the new HTML. We've never done that for other changes, either. I think it would be nice if we can solve that in some generic way, but at this time it wouldn't be worth the effort for those few minutes.

In theory, for this particular change, we could avoid it by deploying the stylesheet changes at least 5 minutes before the rest of the changes.

Jon's patch https://gerrit.wikimedia.org/r/445300 actually fixes the double button problem, although it's a bit unintuitive why (or perhaps a bit of a lucky coincidence). I tried to explain it on the patch. It doesn't even need to be deployed before other changes.

I think we should merge it, even though we usually ignore these five-minute issues: this is a relatively visible area, the patch is tiny and easy to support/remove later, and (most importantly ;) ) is has already been written and reviewed.

Change 446062 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] Set role=button on TOC show/hide checkbox

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

In T195053#4417888, @matmarex wrote:

In T195053#4417543, @stjn wrote:

2. Is CSS-based solution entirely accessible? Here it says that IE11 doesn’t read CSS content.

I don't have Jaws or NVDA, but with Windows Narrator, it indeed doesn't read it. I am not sure if it is worth the effort to support this browser.

We should probably add role="button" to the <input type="checkbox"> though!

Updated test comparison table with VoiceOver/MacOS:

(added Jul 16: Firefox gives the same results as Edge; Chrome doesn't seem to work with Narrator)

Change 446062 merged by jenkins-bot:
[mediawiki/core@master] Set role=button on TOC show/hide checkbox

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

In T195053#4428186, @matmarex wrote:

Jon's patch https://gerrit.wikimedia.org/r/445300 actually fixes the double button problem, although it's a bit unintuitive why (or perhaps a bit of a lucky coincidence). I tried to explain it on the patch. It doesn't even need to be deployed before other changes.

If we deploy the new TOC and this patch at the same time, I expect it will do the following:

• First 5 minutes:
  • A) Old HTML remains viewed with old JS/CSS module list, and old contents of those modules. Neither patch is in effect.
  • B) New HTML on recently saved pages will also load "mediawiki.toc.styles" module, changes the style url, bypasses cache, gets new CSS. Still has old JS for now.
• After 5 minutes:
  • C) Old html gets newer version of "mediawiki.toc" JS and newer version of any CSS request it has (will not load "mediawiki.toc.styles").
  • D) New HTML gets newer version of "mediawiki.toc" JS.

As I understand it, the problem is that use case B (new HTML/CSS, old JS) will cause duplicate buttons to be created. I agree that this patch (updating the CSS) should prevent that.

EDIT: I mistakenly thought the patch adds CSS to mediawiki.toc.styles, but it adds it to mediawiki.toc (the JS module). See T195053#4429441.

However it seems @Fomafix has a different experience. I'm fine with merging, but I'd really like to get confirmation from @Fomafix, given this has been a major project that they have worked on for a year. So if @Fomafix prefers to revert it for now, or to think about it some more, I'd be inclined to let us do that. That way, it can also be a learning experience.

I tested https://gerrit.wikimedia.org/r/445300 as cherry-pick again. It does not prevent double toggle buttons.

The style

.toctogglespan ~ .toctoggle {
	display: none;
}

in resources/src/mediawiki.toc.styles/screen.less instead of resources/src/mediawiki.toc/toc.css to be in the module mediawiki.toc.style instead of mediawiki.toc prevents the double toggle buttons.

New modules gets loaded immediately. Changed modules gets loaded up to 5 minutes later. Is this a bug in ResourceLoader?

In T195053#4429441, @Fomafix wrote:

I tested https://gerrit.wikimedia.org/r/445300 as cherry-pick again. It does not prevent double toggle buttons.

The style

.toctogglespan ~ .toctoggle {
	display: none;
}

in resources/src/mediawiki.toc.styles/screen.less instead of resources/src/mediawiki.toc/toc.css to be in the module mediawiki.toc.style instead of mediawiki.toc would prevent the double toggle buttons.

Excellent. You're absolutely right. I wrongly thought Jon's patch adds CSS to "mediawiki.toc.styles", but the patch currently adds the CSS to "mediawiki.toc". Thus, re-reading T195053#4429164 shows that use case B would get the old JS still (and with that, the old CSS).

In T195053#4429441, @Fomafix wrote:

New modules gets loaded immediately. Changed modules gets loaded up to 5 minutes later. Is this a bug in ResourceLoader?

The difference is not between "new" or "changed" modules. The difference is between stylesheet modules (referenced by URL in the HTML), and dynamic JS/CSS modules (referenced by mw.loader).

This difference is known behaviour and something we choose in 2011. It was a compromise between atomic perfection and simplicity+usability+performance+scalability. In order to keep the system simple for developers, fast in terms of performance, and scalable for Wikipedia – we could not make guarantees between cache-roll of stylesheets and cache-roll of dynamic modules. To change this, would require a significantly more complex system, and we believed it could not perform fast enough for our scale. Or at least, that is what we believed in 2011. Perhaps we should try again :-)

Stylesheet module URLs are cached for 5 minutes. All URLs in older page cache will get new styles within 5 minutes from the same URL. All stylesheets on a single page always come from the same moment in time. ResourceLoader guarantees, that there can not be an older and newer versions mixed of stylesheet modules.

The manifest for mw.loader is also cached for 5 minutes. And any page, both older and newer will get a new manifest within 5 minutes. All dynamically loaded modules always come from the same moment in time. There is no mix of new/old changed modules.

These two systems are not related and will expire at different times. In the case of the TOC change, for newly edited pages, the stylesheet URL will expire first.

Change 445300 merged by jenkins-bot:
[mediawiki/core@master] Cached HTML/JS shouldn't show two toggle links

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

Thank you for looking into this, @Krinkle @Fomafix! I also missed that the styles were in the wrong module.

I think we're all done here. The only outstanding patch https://gerrit.wikimedia.org/r/445258 is kind of unrelated (the purpose of it is to keep the width of the TOC box when it is collapsed).

Looks good to me. I did notice that the [hide] button does not appear on IE8. I don't know if that is expected.

IE8

IE8 does not support :checked and therefor can not supported. The previous JavaScript-based implementation (mediawiki.toc) also did not support IE8 because for IE8 all JavaScript is disabled.

Re: 68527cf47935

Can anyone check the new "show" or "hide" label on IE11 + screen reader? (I don't have an IE11 capable machine.)

IE11 + screen reader reportedly does not speak :before or :after generated content. So a user who tabs to the label after "Contents" might be getting silence instead of "show" or "hide" - a potential "empty form label" accessibility error.

In T195053#4508609, @MattFitzpatrick wrote:

Re: 68527cf47935

Can anyone check the new "show" or "hide" label on IE11 + screen reader? (I don't have an IE11 capable machine.)

IE11 + screen reader reportedly does not speak :before or :after generated content. So a user who tabs to the label after "Contents" might be getting silence instead of "show" or "hide" - a potential "empty form label" accessibility error.

@ABorbaWMF, @Ryasmeen - could one of you look into this?

@MattFitzpatrick We discussed it earlier on the task: Windows Narrator with IE 11 will unfortunately read the show/hide button only as "Off; button" / "On; button". I don't know if more advanced screen readers can handle it better. With Edge, it works better, being read as "'hide'; button" / "'show'; button".

In T195053#4417888, @matmarex wrote:

In T195053#4417543, @stjn wrote:

2. Is CSS-based solution entirely accessible? Here it says that IE11 doesn’t read CSS content.

I don't have Jaws or NVDA, but with Windows Narrator, it indeed doesn't read it. I am not sure if it is worth the effort to support this browser.

We should probably add role="button" to the <input type="checkbox"> though!

Here's how Windows Narrator reads it:

	Old version	Current version	With role="button" added
IE 11	"'hide'; button" / "'show'; button"	"Unchecked checkbox"	"Off; button" / "On; button"
Edge	"'hide'; button" / "'show'; button"	"Unchecked 'hide' checkbox" / "Checked 'show' checkbox"	"'hide'; button" / "'show'; button"

(added Jul 16: Firefox gives the same results as Edge; Chrome doesn't seem to work with Narrator)

Change 481541 had a related patch set uploaded (by Fomafix; owner: Fomafix):
[mediawiki/core@master] Revert "Cached HTML/JS shouldn't show two toggle links"

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

Change 481541 merged by jenkins-bot:
[mediawiki/core@master] Revert "Cached HTML/JS shouldn't show two toggle links"

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

Change 445258 abandoned by Jdlrobson:
Expanding/Collapsing table of contents should not cause reflow

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