Page MenuHomePhabricator

mw.config.get('debug') is not Boolean any more
Closed, ResolvedPublic

Description

According to https://www.mediawiki.org/wiki/Manual:Interface/JavaScript the value of mw.config.get('debug') should be a Boolean (true or false). Right now, it is 0 or 1.

We have some user scripts that checked this and used to expect true or false. It seems like someone has changed it at some point, but did not update the documentation.

The goal of this task is to:

  • Identify the patch in which this change happened
  • Understand the reason behind this change (it is not backward compliant)
  • Ensure the change was properly communicated (e.g. through User-notice) and if not, do so
  • Update the documentation on MediaWiki wiki

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Identify the patch in which this change happened

rMWb04ec0dfcac1: resourceloader: Add internal handling for debug=2

Understand the reason behind this change

See T85805: ResourceLoader: Redesign how debug mode operates and expect the value in future to be 2, not just 1 or 0.

(it is not backward compliant)

It is for normal code using the usual ! or !! operators. Code trying for strict equivalence on a boolean (in JavaScript?!) would have broken, yes. In general it is sadly very risky to assume stability in that the way variables are mangled by PHP might turn up locally after a network fetch (see for example user options).

Ensure the change was properly communicated (e.g. through User-notice) and if not, do so

New debug mode will no doubt be communicated when it's ready. I'm not sure this is an interesting-enough change to waste tens of thousands of people's time with.

Update the documentation on MediaWiki wiki

Sure, good point; done: https://www.mediawiki.org/w/index.php?diff=4458891&oldid=4449944&title=Manual:Interface/JavaScript&diffmode=source

Huji assigned this task to Jdforrester-WMF.

Thanks for the quick response!

(it is not backward compliant)

It is for normal code using the usual ! or !! operators. Code trying for strict equivalence on a boolean (in JavaScript?!) would have broken, yes. In general it is sadly very risky to assume stability in that the way variables are mangled by PHP might turn up locally after a network fetch (see for example user options).

If the only use case was in conditional statements, then yes. But at least in one case, the value of this has been used as the key for an object and this caused the code to stop working. See https://github.com/he7d3r/mw-gadget-DebugModeToggle/commit/7cc3d84f7377cc235e4ae87580e55993f3dd70e5 (specifically, lines 30 and 31).

Ensure the change was properly communicated (e.g. through User-notice) and if not, do so

New debug mode will no doubt be communicated when it's ready. I'm not sure this is an interesting-enough change to waste tens of thousands of people's time with.

Update the documentation on MediaWiki wiki

Sure, good point; done: https://www.mediawiki.org/w/index.php?diff=4458891&oldid=4449944&title=Manual:Interface/JavaScript&diffmode=source

Thanks; I further edited that page in https://www.mediawiki.org/w/index.php?diff=next&oldid=4458891&diffmode=source and included a link to the original phab task.

I think you may be underestimating the impact this can have (see example above) and I see no harm in communicating this change. Given that it was merged around one month ago ([https://gerrit.wikimedia.org/r/c/mediawiki/core/+/597912 mid-Feburary 2021]) it may be a bit too late now, but I think backward incompatible changes to commonly used config variables are generally worth an announcement in UserNotice.

Anyway, at this point no further action is needed. Closing this task as complete.

Thanks for the quick response!

Happy to help. Sorry this bit you.

(it is not backward compliant)

It is for normal code using the usual ! or !! operators. Code trying for strict equivalence on a boolean (in JavaScript?!) would have broken, yes. In general it is sadly very risky to assume stability in that the way variables are mangled by PHP might turn up locally after a network fetch (see for example user options).

If the only use case was in conditional statements, then yes. But at least in one case, the value of this has been used as the key for an object and this caused the code to stop working. See https://github.com/he7d3r/mw-gadget-DebugModeToggle/commit/7cc3d84f7377cc235e4ae87580e55993f3dd70e5 (specifically, lines 30 and 31).

Oh wow, yes, one should never be programmatically interacting with debug mode on production servers. It's for manual debugging, and these kinds of changes of internal characteristics are subject to change without notice. This kind of user script is a really bad idea.

Update the documentation on MediaWiki wiki

Sure, good point; done: https://www.mediawiki.org/w/index.php?diff=4458891&oldid=4449944&title=Manual:Interface/JavaScript&diffmode=source

Thanks; I further edited that page in https://www.mediawiki.org/w/index.php?diff=next&oldid=4458891&diffmode=source and included a link to the original phab task.

Thank you.

[…] one should never be programmatically interacting with debug mode on production servers. It's for manual debugging, and these kinds of changes of internal characteristics are subject to change without notice. This kind of user script is a really bad idea.

While this was indeed not a supported way of interacting with debug mode, I think this user script is actually quite a good idea!

User scripts don't have to be stable or limited to using officially supported APIs – that's part of what makes them minimal to make and so accessible. So long as it is understood by editors that it may break, and that we can't always help you fix issues, then having it sometimes not work while you wait for a fellow editor to make a fix, might actually be a very worthwhile trade-off (compared to the cost-benefit of doing it "properly", if and when possible).