Page MenuHomePhabricator

safemode parameter doesn't work with VE
Closed, ResolvedPublic8 Estimated Story Points

Description

Problem

Adding safemode=1 to the URL should prevent user scripts, gadgets, etc. from loading. While this works in most cases, it does not work while editing in VE:

Several ways to reproduce:

  1. Open https://de.wikipedia.org/wiki/Benutzer:Schnark?safemode=1 (scripts not loaded as expected)
  2. Click "Edit".

The URL changes to https://de.wikipedia.org/wiki/Benutzer:Schnark?safemode=1&veaction=edit, but unexpectedly my user scripts do load.

The same happens when:

[And also, NWE doesn't load for me without displaying any error, but per above I'm not sure whether one of my scripts is responsible for this or not.]

Event Timeline

Change 405992 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] OutputPage: Make filterModules() public

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

Change 405993 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] Support 'safemode' parameter

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

Jdforrester-WMF set the point value for this task to 8.

I'm going to evaluate the feasibility of addressing this from ResourceLoader's perspective instead.

Requirement:

  • Make it so that a calling mw.loader.using(module) under module does not load the module when in safemode.

Restriction:

  • Preserve the fact that client-side is not aware and not responsible for this feature. (Similar to how client is not aware of the other reality filters, like targetT127268).

Proposal 1

That essentially leaves only one option, which is to pass safemode from OutputPage to load.php?modules=startup, and filter the modules out from the manifest. That way, everything will naturally work.

Benefits:

  • Works for all customers, including core, VisualEditor, and anything else that might call into mw.loader. (At least one other one I know of: WikiLove's local module)
  • Simplifies OutputPage by not having to filter modules everywhere.

Cost:

  • Converts the feature from being isolated to MediaWiki OutputPage, to now being part of ResourceLoader's feature set.
  • We'll still have at least one place outside StartupModule dealing with safemode, which is the styles module queue. Either in OutputPage, or in ResourceLoaderClientHtml.

In terms of cost and fitting with ResourceLoader long-term, I don't see it as a burden because it would be a natural fit alongside the existing "origin" attributes we have in the ResourceLoaderModule interface. Safemode is just way to filter the registry based on that attribute.

Change 405992 abandoned by Krinkle:
OutputPage: Make filterModules() public

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

Change 405993 abandoned by Bartosz Dziewoński:
Support 'safemode' parameter

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

@Krinkle Thanks for picking this up. Your proposal makes more sense than what I imagined when reading your comments on patch 405992 :)

There is one caveat to handling this similarly to the targets system – these modules will appear not to exist on some pages, which will break modules that have static dependencies on them. I think this is reasonable, but we'll need to document it as a breaking change. From a quick search, the only affected module in our repos is 'ext.flow.visualEditor'.

Change 422357 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] resourceloader: Replace ClientHtml 'target' param with 'options' array

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

Change 422358 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] [WIP] resourceloader: Apply safemode to startup module registry

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

Change 422357 merged by jenkins-bot:
[mediawiki/core@master] resourceloader: Replace ClientHtml 'target' param with 'options' array

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

Progressing the safemode patch (https://gerrit.wikimedia.org/r/422358) is currently blocked on the excludepage patch (https://gerrit.wikimedia.org/r/340768), which in turn is blocked on a "group=user module" patch (https://gerrit.wikimedia.org/r/416619).

Change 429872 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] VisualEditorDataModule: Remove origin restriction

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

Change 429873 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/Cite@master] CiteDataModule: Remove origin restriction

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

Change 429876 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/Citoid@master] CitoidDataModule: Remove origin restriction

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

Change 429878 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/Kartographer@master] Kartographer\DataModule: Remove origin restriction

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

Change 429951 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/Kartographer@master] Kartographer\DataModuleLinks: Remove origin restriction

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

Change 429976 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/Math@master] MathMathSymbolsDataModule: Remove origin restriction

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

Change 429977 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/Math@master] MathChemSymbolsDataModule: Remove origin restriction

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

Change 429978 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/PageTriage@master] PageTriageMessagesModule: Remove origin restriction

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

Change 429873 merged by jenkins-bot:
[mediawiki/extensions/Cite@master] CiteDataModule: Remove origin restriction

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

Change 429872 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] VisualEditorDataModule: Remove origin restriction

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

Change 429876 merged by jenkins-bot:
[mediawiki/extensions/Citoid@master] CitoidDataModule: Remove origin restriction

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

Change 429976 merged by jenkins-bot:
[mediawiki/extensions/Math@master] MathMathSymbolsDataModule: Remove origin restriction

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

Change 429977 merged by jenkins-bot:
[mediawiki/extensions/Math@master] MathChemSymbolsDataModule: Remove origin restriction

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

Change 429878 merged by jenkins-bot:
[mediawiki/extensions/Kartographer@master] Kartographer\DataModule: Remove origin restriction

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

Change 429951 merged by jenkins-bot:
[mediawiki/extensions/Kartographer@master] Kartographer\DataModuleLinks: Remove origin restriction

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

Change 429978 merged by jenkins-bot:
[mediawiki/extensions/PageTriage@master] PageTriageMessagesModule: Remove origin restriction

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

Change 429981 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/Flow@master] Remove 'site' and 'user' modules as static dependencies

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

Change 429981 merged by jenkins-bot:
[mediawiki/extensions/Flow@master] Remove 'site' and 'user' modules as static dependencies

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

[mediawiki/core@master] resourceloader: Apply safemode to startup module registry
https://gerrit.wikimedia.org/r/422358

@matmarex Is that all of them? If so, then this is un-stalled now I think.

Yeah, all of my extension patches were merged. I'm not aware of any more issues that your ResourceLoader patch could cause.

Change 422358 merged by jenkins-bot:
[mediawiki/core@master] resourceloader: Apply safemode to startup module registry

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

Re-assigning given the ResourceLoader part seems done. Please verify whether this solves the issue for VE. Let me know if something else comes up or if it doesn't work as expected.

matmarex reassigned this task from matmarex to Krinkle.

Yes, that fixes it for VE.

(Sorry for the delay, this got lost in my inbox. Thanks for assigning to me, otherwise it would have been lost for longer ;) )