Page MenuHomePhabricator
Create Task
Maniphest T140804

Remove $wgResourceLoaderLESSVars and ResourceLoaderGetLessVars hook
Closed, ResolvedPublic
Actions

Description

It's almost entirely unused. Instead we have two other approaches:

  • Explicit import from core-provided modules (e.g. @import "mediawiki.ui/variables";).
  • Implicit import from currently active skin or theme (not yet done, but under consideration for T112747).

Problems with $wgResourceLoaderLESSVars:

  • Inherently lead to unmaintainable code. There is no explicit link between code stuffing variables in there (e.g. site configuration, extensions) and code consuming those variables. This makes it very fragile to change or migrate anything.
  • Changing anything in there also results in immediate global invalidation of all Less-related caches because anything could be using it without knowing.

The last known use of it was to introduce a global variable deviceWidthTablet (T93675; 48263c3). Once that is moved to variables.less file and explicitly imported by callers, we can remove $wgResourceLoaderLESSVars from core.

Details

SubjectRepoBranchLines +/-
resourceloader: Drop deprecated ResourceLoader::getLessVarsmediawiki/coremaster+2 -12
resourceloader: Remove $wgResourceLoaderLESSVars supportmediawiki/coremaster+24 -77
Use @width-breakpoint-tabletmediawiki/extensions/MobileFrontendmaster+5 -5
Use @width-breakpoint-tablet instead of @deviceWidthTabletmediawiki/skins/Vectormaster+2 -1
search: Use @width-breakpoint-tablet instead of @deviceWidthTabletmediawiki/coremaster+1 -1
resourceloader: Remove deprecated ResourceLoaderGetLessVars hookmediawiki/coremaster+5 -10
resourceloader: Soft-deprecate use of global LESS variablesmediawiki/coremaster+9 -10
Avoid deprecated ResourceLoaderGetLessVars hookmediawiki/extensions/XenForoAuthmaster+22 -18
Customize query in gerrit

Related Objects
Search...

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Restricted Application added subscribers: Zppix, Aklapper. · View Herald TranscriptJul 19 2016, 6:56 PM
Krinkle mentioned this in T140807: Remove $wgResourceLoaderLESSImportPaths.Jul 19 2016, 7:25 PM
Krinkle moved this task from Inbox to Backlog on the MediaWiki-ResourceLoader board.Jul 19 2016, 7:28 PM
Krinkle moved this task from Unsorted to Needs removal on the Technical-Debt board.Jul 19 2016, 7:31 PM
matmarex subscribed.Jul 19 2016, 10:55 PM

We also have the ResourceLoaderGetLessVars hook, which implements the exact same functionality.

We also allow subclassing ResourceLoaderModule to override getLessVars(), which can be used to achieve the same, too (requires more code, but produces less unmaintainable results).

Krinkle added a comment.EditedJul 19 2016, 11:08 PM
In T140804#2477884, @matmarex wrote:

We also have the ResourceLoaderGetLessVars hook, which implements the exact same functionality.

We also allow subclassing ResourceLoaderModule to override getLessVars(), which can be used to achieve the same, too (requires more code, but produces less unmaintainable results).

Yeah, the ResourceLoaderModule method is fine. We should remove the hook though. Forgot about that one!

Krinkle changed the task status from Open to Stalled.Apr 11 2017, 12:18 AM
Krinkle triaged this task as Medium priority.
Krinkle added a subtask: T112747: Devise a generic way for theme-agnostic stylesheets to adapt to the current theme.Apr 11 2017, 12:45 AM
Krinkle removed a subtask: T112747: Devise a generic way for theme-agnostic stylesheets to adapt to the current theme.
SamanthaNguyen mentioned this in T162653: Refreshed should use core-provided tablet width variable.Apr 11 2017, 4:14 AM
Krinkle renamed this task from Remove $wgResourceLoaderLESSVars to Remove $wgResourceLoaderLESSVars and ResourceLoaderGetLessVars hook.Jul 19 2017, 11:36 PM
Krinkle mentioned this in T169826: Add project wordmark to print styles.
• bmansurov subscribed.Jul 20 2017, 1:14 PM

When is a good time to update the documentation to discourage users from using the hook?

Krinkle changed the task status from Stalled to Open.Jul 21 2017, 11:17 PM
Krinkle moved this task from Backlog to Accepted Enhancement on the MediaWiki-ResourceLoader board.
gerritbot subscribed.Jul 22 2017, 12:33 AM

Change 366990 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] resourceloader: Soft-deprecate use of global LESS variables

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

gerritbot added a project: Patch-For-Review.Jul 22 2017, 12:33 AM
Krinkle created subtask T171365: Avoid ResourceLoaderGetLessVars hook in MobileFrontend by adding mobile threshold to Less variables in core (MFDeviceWidthMobileSmall).Jul 22 2017, 12:45 AM
gerritbot added a comment.Jul 22 2017, 12:55 AM

Change 366992 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/extensions/XenForoAuth@master] Avoid deprecated ResourceLoaderGetLessVars hook

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

Krinkle created subtask T171367: Avoid ResourceLoaderGetLessVars hook in MinervaNeue.Jul 22 2017, 1:21 AM
Krinkle mentioned this in rEXFA248f49e4f632: Avoid deprecated ResourceLoaderGetLessVars hook.Jul 22 2017, 2:19 AM
Krinkle mentioned this in rEXFAd4e64cc836c0: Avoid deprecated ResourceLoaderGetLessVars hook.
Krinkle mentioned this in rEXFA3505f79747cf: Avoid deprecated ResourceLoaderGetLessVars hook.Jul 26 2017, 12:52 AM
gerritbot added a comment.Jul 26 2017, 8:14 AM

Change 366992 merged by jenkins-bot:
[mediawiki/extensions/XenForoAuth@master] Avoid deprecated ResourceLoaderGetLessVars hook

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

gerritbot added a comment.Jul 28 2017, 5:09 PM

Change 366990 merged by jenkins-bot:
[mediawiki/core@master] resourceloader: Soft-deprecate use of global LESS variables

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

matmarex removed a project: Patch-For-Review.Jul 28 2017, 5:41 PM
ReleaseTaggerBot added a project: MW-1.30-release-notes.Jul 28 2017, 6:00 PM
Krinkle removed a project: MW-1.30-release-notes.Sep 20 2017, 7:18 PM
Liuxinyu970226 subscribed.Dec 6 2017, 12:29 PM
Restricted Application added a project: Performance-Team. · View Herald TranscriptDec 6 2017, 12:29 PM
Legoktm mentioned this in T161868: extension.json / skin.json: Overriding of values, e.g. "ResourceLoaderLESSVars".Dec 6 2017, 9:15 PM
• Imarlier moved this task from Inbox, needs triage to Radar on the Performance-Team board.Dec 11 2017, 8:57 PM
• Imarlier edited projects, added Performance-Team (Radar); removed Performance-Team.
Krinkle changed the task status from Open to Stalled.Dec 13 2017, 6:19 PM
Krinkle edited projects, added Performance-Team; removed Performance-Team (Radar).
Krinkle moved this task from Inbox, needs triage to Blocked (old) on the Performance-Team board.
Jdlrobson closed subtask T171365: Avoid ResourceLoaderGetLessVars hook in MobileFrontend by adding mobile threshold to Less variables in core (MFDeviceWidthMobileSmall) as Resolved.Mar 6 2018, 5:55 PM
Krinkle reopened subtask T171365: Avoid ResourceLoaderGetLessVars hook in MobileFrontend by adding mobile threshold to Less variables in core (MFDeviceWidthMobileSmall) as Open.Apr 9 2018, 7:40 PM
Krinkle closed subtask T171367: Avoid ResourceLoaderGetLessVars hook in MinervaNeue as Resolved.
Liuxinyu970226 unsubscribed.Apr 10 2018, 11:40 AM
Krinkle changed the task status from Stalled to Open.Apr 10 2018, 1:58 PM
Krinkle closed subtask T171365: Avoid ResourceLoaderGetLessVars hook in MobileFrontend by adding mobile threshold to Less variables in core (MFDeviceWidthMobileSmall) as Resolved.
Krinkle moved this task from Blocked (old) to To-do: Goals prioritized current Quarter on the Performance-Team board.
Krinkle removed a project: MediaWiki-User-Interface.
gerritbot added a comment.Apr 10 2018, 2:11 PM

Change 425273 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] resourceloader: Remove deprecated ResourceLoaderGetLessVars hook

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

gerritbot added a project: Patch-For-Review.Apr 10 2018, 2:11 PM
Jdlrobson added a comment.Apr 10 2018, 2:44 PM

I've tested and merged the above patch. Removing $wgResourceLoaderLESSVars will need a little more work however.

gerritbot added a comment.Apr 10 2018, 2:51 PM

Change 425273 merged by jenkins-bot:
[mediawiki/core@master] resourceloader: Remove deprecated ResourceLoaderGetLessVars hook

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

ReleaseTaggerBot added a project: MW-1.31-release-notes (WMF-deploy-2018-04-10 (1.31.0-wmf.29)).Apr 10 2018, 3:00 PM
Krinkle removed projects: MW-1.31-release-notes (WMF-deploy-2018-04-10 (1.31.0-wmf.29)), Patch-For-Review.Apr 10 2018, 7:25 PM
Krinkle changed the task status from Open to Stalled.Apr 10 2018, 7:27 PM
Krinkle moved this task from To-do: Goals prioritized current Quarter to Blocked (old) on the Performance-Team board.
Krinkle mentioned this in T191937: ResourceLoader LESS cache should vary on wiki-global vars (aside from module own vars).Apr 11 2018, 12:11 AM
Krinkle added a comment.EditedApr 11 2018, 12:40 AM
In T140804#4120405, @Jdlrobson wrote:

I've tested and merged the above patch. Removing $wgResourceLoaderLESSVars will need a little more work however.

The only remaining use of wgResourceLoaderLESSVars is MobileFrontend reading core's deviceWidthTablet key, which is a deprecated copy of @width-breakpoint-tablet from mediawiki.ui/variables.less. As much as it seems odd, I think it may be best to hardcode that number in MobileFrontend.hooks.php for the moment.

It will remain a duplicate, but in a different place. E.g. moving being a copy in core/DefaultSettings.php to being a copy in MobileFrontend.

I wouldn't mind prolonging this further, but only if we have a plan.

As it stands, I don't think there's a way to solve this. I assume we don't want MobileFrontend PHP to parse LESS-syntaxed files from MediaWiki core internals to extract a variable. Currently MF uses it via mobile.startup/browser.js#isWideScreen, which is accessed by:

  1. mobile.special.mobileoptions.scripts/mobileoptions.js: Hides part of an interface. Could this use CSS display:none instead, with the original Less variable directly (and e.g. a media query).
  2. mobile.startup/PageList.js: Uses it as a proxy to decide "Delay an unnecessary load of images on mobile connections". Seems obsolete with the skin separation as MF is/should be mobile-only. Could this be replaced with a browser.isMobile() method of some sort that isn't related to tablet-width, but is instead based on browser features, or user-agent, or wgMFMode?
  3. mobile.toggle/toggle.js: Decides whether to remember a decision to expand a section. Based on comments from mobileoptions.js, it seems unexpected that a section could be toggled at this point, given its interface was meant to be hidden. Either way, both ways can be solved in CSS. The toggle button can be hidden with CSS if needed. And if there are other ways to toggle a section that need to be no-ops for some reason, then the toggling itself can use a class (instead of inline display:none styles), and that class can toggle display:none from a media query. Should not affect performance.
• bmansurov unsubscribed.Apr 11 2018, 2:14 AM
Krinkle edited projects, added Technical-Debt (Deprecation process); removed Technical-Debt.Jul 13 2018, 11:11 PM
Krinkle moved this task from Untriaged to 2017 | MW 1.29 / 1.30 on the Technical-Debt (Deprecation process) board.Jul 13 2018, 11:14 PM
Krinkle mentioned this in T199854: Remove use of deprecated $wgResourceLoaderLESSVars in MobileFrontend.Jul 18 2018, 4:25 AM
Krinkle changed the task status from Stalled to Open.Jul 25 2018, 4:58 PM
Krinkle closed subtask T199854: Remove use of deprecated $wgResourceLoaderLESSVars in MobileFrontend as Resolved.
Krinkle moved this task from Blocked (old) to To-do: Goals prioritized current Quarter on the Performance-Team board.
gerritbot added a comment.Aug 9 2018, 3:20 PM

Change 451648 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] resourceloader: Remove $wgResourceLoaderLESSVars support

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

gerritbot added a project: Patch-For-Review.Aug 9 2018, 3:20 PM
gerritbot added a comment.Aug 9 2018, 4:56 PM

Change 451661 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] search: Use @width-breakpoint-tablet instead of @deviceWidthTablet

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

gerritbot added a comment.Aug 9 2018, 4:57 PM

Change 451662 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/skins/Vector@master] Use @width-breakpoint-tablet instead of @deviceWidthTablet

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

gerritbot added a comment.Aug 9 2018, 5:18 PM

Change 451662 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Use @width-breakpoint-tablet instead of @deviceWidthTablet

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

ReleaseTaggerBot added a project: MW-1.32-notes (WMF-deploy-2018-08-14 (1.32.0-wmf.17)).Aug 9 2018, 6:01 PM
gerritbot added a comment.Aug 9 2018, 6:30 PM

Change 451661 merged by jenkins-bot:
[mediawiki/core@master] search: Use @width-breakpoint-tablet instead of @deviceWidthTablet

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

gerritbot added a comment.Aug 9 2018, 10:36 PM

Change 451801 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Use @width-breakpoint-tablet

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

matmarex unsubscribed.Aug 9 2018, 10:38 PM
gerritbot added a comment.Aug 10 2018, 2:14 AM

Change 451801 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Use @width-breakpoint-tablet

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

Krinkle claimed this task.Aug 12 2018, 12:51 PM
Krinkle moved this task from To-do: Goals prioritized current Quarter to Doing (old) on the Performance-Team board.
Jdforrester-WMF edited projects, added MW-1.32-notes (WMF-deploy-2018-08-21 (1.32.0-wmf.18)); removed MW-1.32-notes (WMF-deploy-2018-08-14 (1.32.0-wmf.17)).Aug 14 2018, 8:55 PM
gerritbot added a comment.Aug 15 2018, 1:24 AM

Change 451648 merged by jenkins-bot:
[mediawiki/core@master] resourceloader: Remove $wgResourceLoaderLESSVars support

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

Krinkle closed this task as Resolved.Aug 15 2018, 2:08 AM
Krinkle removed a project: Patch-For-Review.
Jdlrobson mentioned this in T202021: Regression: Notifications, Special:MobileOptions broken!.Aug 15 2018, 7:00 PM

This has caused a serious regression -https://phabricator.wikimedia.org/T202021 :/
Why was this not picked up in CI?
Is there an issue with mobile skin CI being run in core?

Krinkle added a comment.Aug 15 2018, 7:47 PM
In T140804#4505465, @Jdlrobson wrote:

Why was this not picked up in CI?

It seems that the shared extension "gate" job does not install MinervaNeue. It installs MobileFrontend and a handful of other common extensions, but not MinervaNeue. Seems worth adding!

Jdlrobson mentioned this in T202030: CI: Minerva PHPUnit tests should be included in shared extension gate job.Aug 15 2018, 9:47 PM
• Niedzielski mentioned this in rEQS9bf950c37f26: Fix: replace deviceTabletWidth w/ width-breakpoint-tablet in LESS.Aug 16 2018, 2:35 PM
Umherirrender mentioned this in T202125: BlueSpicePageAssignments and BlueSpiceSkin using removed "ResourceLoaderLESSVars" config.Aug 17 2018, 9:35 AM
Noloader subscribed.EditedJun 7 2019, 8:40 PM

This option is still needed for Mobile FrontEnd.

In fact, we needed to add it to our LocalSettings.php after a 1.31.1 -> 1.31.2 upgrade. Also see undefined option: 'ResourceLoaderLESSVars'. This post was by another user who needed to add it after a recent migration.

gerritbot added a comment.Oct 9 2019, 10:31 PM

Change 540093 had a related patch set uploaded (by Krinkle; owner: Fomafix):
[mediawiki/core@master] resourceloader: Drop deprecated ResourceLoader::getLessVars

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

gerritbot added a project: Patch-For-Review.Oct 9 2019, 10:31 PM
gerritbot added a comment.Oct 9 2019, 10:43 PM

Change 540093 merged by jenkins-bot:
[mediawiki/core@master] resourceloader: Drop deprecated ResourceLoader::getLessVars

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

ReleaseTaggerBot added a project: MW-1.35-notes (1.35.0-wmf.2; 2019-10-15).Oct 9 2019, 11:00 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL