Page MenuHomePhabricator

Remove use of deprecated $wgResourceLoaderLESSVars in MobileFrontend
Closed, ResolvedPublic

Description

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.

Acceptance criteria

  • Replace lines in MobileFrontend hooks pointing to lessVars - use a local constant instead (use hardcoded value (720px))
'wgMFDeviceWidthTablet' => $lessVars['deviceWidthTablet'],
  • Remove access to config for LESS variables inside onBeforePageDisplay and onResourceLoaderGetConfigVars
$lessVars = $config->get( 'ResourceLoaderLESSVars' );

Details

Related Gerrit Patches:
mediawiki/extensions/MobileFrontend : masterRemove use of deprecated $wgResourceLoaderLESSVars in MF ext

Event Timeline

Krinkle triaged this task as Medium priority.Jul 18 2018, 4:25 AM
Krinkle created this task.
Jdlrobson updated the task description. (Show Details)

Change 447792 had a related patch set uploaded (by D3r1ck01; owner: Alangi Derick):
[mediawiki/extensions/MobileFrontend@master] Remove use of deprecated $wgResourceLoaderLESSVars in MF ext

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

Adding to kanbanana board so we can track and help review @D3r1ck01's patch!

Thanks @Jdlrobson, so this is still to be kept open until a final solution is worked on?

Jdlrobson removed a project: Patch-For-Review.

Skipping QA/design review as this shouldn't be a visible change.
@D3r1ck01 I'll let @Krinkle confirm this is fixed now. @Krinkle can this be resolved?

Change 447792 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Remove use of deprecated $wgResourceLoaderLESSVars in MF ext

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

Krinkle closed this task as Resolved.Jul 25 2018, 4:58 PM

Looks good to me. Thanks!