Page MenuHomePhabricator

Mobile target cannot have a default enabled gadget for anonymous users
Open, NormalPublic

Description

I tried launching some functionality for mobile by making use of a default gadget, but apparently (likely because mobile target replaces/modifies the site module), this is not possible.

This is inconsistent and unexpected.

Event Timeline

TheDJ created this task.Aug 14 2017, 1:50 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 14 2017, 1:50 PM
Jdlrobson moved this task from Needs triage to Triaged on the Mobile board.Aug 15 2017, 2:16 PM
Jdlrobson removed a project: MobileFrontend.

MobileFrontend just uses the target system.

@TheDJ default gadgets for anonymous users scare me from a performance, UI consistency and debugging point of view. They seem like an opportunity to sneak functionality into the software that doesn't go through our normal quality checks. This may be a sensible default if intentional.

for future reference, a workaround is using mw.loader.load( 'ext.gadget.itsname' ) from MediaWiki:Mobile.js

@TheDJ default gadgets for anonymous users scare me from a performance, UI consistency and debugging point of view. They seem like an opportunity to sneak functionality into the software that doesn't go through our normal quality checks. This may be a sensible default if intentional.

No one is trying to "sneak" anything in - this is the whole point of the Gadgets extension, to give wiki admins extra control over JS/CSS, for better or for worse. And as TheDJ pointed out, there are easy work arounds to enable this functionality, so we should just fix this properly.

Sure, but most gadgets are usually minor enhancements. This is a pretty big one. We wouldn't ship VisualEditor or Echo as a gadget would we (I hope not!)? :-) What are the reasons for enabling this for all anons as a gadget? I'm confused as I thought gadgets were for innovating and battle testing ideas and this makes total sense for logged in users, but if we want to roll out to all mobile anons, I don't understand why the code cannot be ported to an extension where we can add browser tests/unit tests and make it impossible for us to break this feature. Anons also cannot give feedback for software and while we're planning to collect JavaScript errors for our users (T167699) we've not quite got round to it yet, so if there are bugs we will not know about them.

One big problem with the existing gadget is it is making assumptions about the HTML and layout. We've already made changes to that this week. HTML is not an API. if we want to encourage adding gadgets to this menu, it would really benefit from a true API/hook.

TheDJ renamed this task from Mobile target cannot have a default enabled gadget for anonymous users to Minerva does not support having a default enabled gadget.Aug 24 2017, 8:56 PM
TheDJ triaged this task as Normal priority.
TheDJ added a project: MinervaNeue.
TheDJ updated the task description. (Show Details)
TheDJ added a comment.Aug 24 2017, 8:59 PM

One big problem with the existing gadget is it is making assumptions about the HTML and layout.

Less of a big problem than you might think. We change libraries and template names etc, just as often, if not more often.
For instance in the last deprecations, we had some ancient 12 year old userscripts, that predated us even having libraries, and those all kept working, while some of our 10 year old scripts making use of libraries were terribly broken and outdated.

Anyway, that doesn't have so much to do with this specific ticket.

[edit] I spent some more time on this on test2 and figure out that this also doesn't work when Minerva is used in desktop target mode.

Are you sure? It seems super weird to me that Minerva would be the problem. It doesn't touch anything to do with gadgets. I would have guessed this was an issue with the target mode.
Have you verified whether it loads on https://test2.m.wikipedia.org/?useskin=vector&useformat=mobile (Vector in target mode mobile)?

How do you enable a gadget on a skin? Could this be because you are using the wrong key somewhere due to the "minerva" or "minervaneue" confusion?

I'm getting "Error: Unknown dependency: ext.gadget.MobileMaps Error" on Vector skin for English Wikipedia right now:
https://en.wikipedia.org/wiki/Spain?useskin=vector

And when I do:

mw.loader.using('ext.gadget.MobileMaps')

on https://test2.wikipedia.org/wiki/Spain?useskin=minervaneue&useformat=desktop I get no exception, so i'm pretty sure this is not Minerva specific.

TheDJ added a comment.Aug 25 2017, 7:26 PM

Eh wait.. so we have two keys for minerva now ? And both load the same skin, both load [[MediaWiki:Minerva.js]] but both load a different set of gadgets ? well that explains..

TheDJ renamed this task from Minerva does not support having a default enabled gadget to Mobile target cannot have a default enabled gadget for anonymous users.Aug 25 2017, 7:27 PM
TheDJ updated the task description. (Show Details)

@TheDJ - it's an unfortunate side effect of pulling Minerva out of MobileFrontend that I'm trying to resolve in T171644 - but hit a few snags.