Page MenuHomePhabricator

safemode parameter doesn't work with VE
Closed, ResolvedPublic8 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

Schnark created this task.Jan 19 2018, 11:03 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 19 2018, 11:03 AM

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

Restricted Application added a project: Performance-Team. · View Herald TranscriptJan 23 2018, 11:58 PM
Jdforrester-WMF triaged this task as Normal priority.Jan 29 2018, 5:30 PM
Jdforrester-WMF set the point value for this task to 8.
Krinkle moved this task from Inbox to Next In This Quarter on the Performance-Team board.
Krinkle updated the task description. (Show Details)Mar 15 2018, 1:11 AM

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.

Krinkle claimed this task.Mar 15 2018, 1:28 AM
Krinkle added a subscriber: matmarex.

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

Krinkle reassigned this task from Krinkle to matmarex.May 19 2018, 11:26 PM

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 closed this task as Resolved.Jun 15 2018, 9:31 PM
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 ;) )

Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptJun 15 2018, 9:31 PM