Page MenuHomePhabricator

Gadget definition "targets=mobile" seem to target mobile and desktop users on ur.wp
Closed, DuplicatePublic

Description

In urdu wikipedia, we noticed here on gadgets definition page that

targets=mobile

targets both mobile and desktop users. While we would like to enable this gadget for mobile users only.

Event Timeline

MuhammadShuaib updated the task description. (Show Details)
MuhammadShuaib raised the priority of this task from to Needs Triage.
MuhammadShuaib added a subscriber: MuhammadShuaib.
Restricted Application added subscribers: JEumerus, StudiesWorld, Matanya, Aklapper. · View Herald TranscriptFeb 17 2016, 5:24 AM

While https://ur.wikipedia.org/wiki/میڈیاویکی:Gadget-NotoNastaleeqMobile.css exists, very similar CSS font definitions are also in https://ur.wikipedia.org/wiki/میڈیاویکی:Common.css .
So I could imagine that the last link defines the fonts and that there is no software bug here. (But I don't know what gets loaded first and is supposed to be "stronger" / overwrite.) Could you confirm?

Aklapper renamed this task from targets=mobile targets mobile and desktop users to Gadget definition "targets=mobile" seem to target mobile and desktop users on ur.wp.Feb 17 2016, 9:07 AM
Aklapper set Security to None.

While https://ur.wikipedia.org/wiki/میڈیاویکی:Gadget-NotoNastaleeqMobile.css exists, very similar CSS font definitions are also in https://ur.wikipedia.org/wiki/میڈیاویکی:Common.css .
So I could imagine that the last link defines the fonts and that there is no software bug here. (But I don't know what gets loaded first and is supposed to be "stronger" / overwrite.) Could you confirm?

On mobile browsers, Gadget should get loaded first and overwrite the fonts defined in common.css.

On mobile browsers, Gadget should get loaded first and overwrite the fonts defined in common.css.

Is that documented somewhere? Link welcome. :)

Ebraminio added a subscriber: Ebraminio.EditedFeb 18 2016, 10:12 AM

Steps to repro:

  1. Open https://ur.wikipedia.org/wiki/خاص:ترجیحات?uselang=qqx#mw-prefsection-gadgets
  2. Open Gadgets tab, make sure (gadget-NotoNastaleeq) is NOT checked but check (gadget-NotoNastaleeqMobile) then save gadgets preference.
  3. Open https://ur.wikipedia.org/wiki/صفحۂ_اول

Actual:
Black background on Urdu Wikipedia with the gadget.

Expected:
No black background on Urdu Wikipedia as ".skin-vector { background-color: black; }" was applied on https://ur.wikipedia.org/wiki/میڈیاویکی:Gadget-NotoNastaleeqMobile.css which is a mobile gadget and not anywhere else, see https://ur.wikipedia.org/wiki/میڈیاویکی:Gadgets-definition which notes NotoNastaleeqMobile targets mobile, "NotoNastaleeqMobile[ResourceLoader|top|targets=mobile]|NotoNastaleeqMobile.css"

Ebraminio added a comment.EditedFeb 19 2016, 10:20 AM

I've moved the test on Test Wikipedia to not confuse Urdu Wikipedia users, so new steps to repro:

  1. Open https://test.wikipedia.org/wiki/Special:Preferences#mw-prefsection-gadgets and enable "Test mobile only gadget bug" and save
  2. Open https://test.wikipedia.org/wiki/Main_Page

Actual:
Black sidebar background

Expected:
It shouldn't be. https://test.wikipedia.org/wiki/MediaWiki:Gadget-mobileonlygadget.css was "mobileonlygadget[ResourceLoader|targets=mobile]|mobileonlygadget.css" on https://test.wikipedia.org/wiki/MediaWiki:Gadgets-definition , perhaps the expected behavior would not applying it on Desktop view.

Krinkle triaged this task as Normal priority.Feb 29 2016, 8:09 PM
Krinkle moved this task from Inbox to Backlog on the MediaWiki-ResourceLoader board.

Looks like maybe Gadgets doesn't implement the requirements for Targets completely. It added support for setting the module object properties but doesn't read it back out when adding modules to the page.

Either that, or the bug is in MediaWiki core OutputPage, where the style queue is missing support for the target restriction (the stylesheet urls lacks target=). This is one of various aspects of the Target system that wasn't fully considered when it was implemented. However this will be obsolete with T127268, so I don't expect these bugs to be addressed in core (unless volunteered).

Per T127268, I'd encourage you to plan ahead for when the targets system doesn't exist anymore.

For example, you may want to use the skins filter instead (e.g. skins=minerva).

Majr added a subscriber: Majr.May 27 2016, 8:40 AM

For example, you may want to use the skins filter instead (e.g. skins=minerva).

Too bad that doesn't work either. Gadgets uses the user's skin setting, which is never minerva.

For example, you may want to use the skins filter instead (e.g. skins=minerva).

Too bad that doesn't work either. Gadgets uses the user's skin setting, which is never minerva.

@Majr was working on it on T140863: Allow mobile-specific gadgets: https://gerrit.wikimedia.org/r/#/c/299950/

I'm not sure if this is still relevant, given this comment: T173309#3554105

Majr added a comment.Mar 10 2018, 11:58 PM

It's a simple fix, but it was stalled waiting review. If the issue is still relevant a year and half later and someone wants to review I'm happy to rebase. The main thing I personally wasn't sure of was if my test was any good or made any sense.

TheDJ added a subscriber: TheDJ.Jul 12 2018, 9:27 PM

It's a simple fix, but it was stalled waiting review. If the issue is still relevant a year and half later and someone wants to review I'm happy to rebase. The main thing I personally wasn't sure of was if my test was any good or made any sense.

Yikes, this still didn't get merged i guess..
I rebased the patch and forked the specific issue to T199478: Gadgets should use the current skin, instead of the skin preference, since it doesn't really fix the targets system, which this ticket is about.