Page MenuHomePhabricator

Unknown dependency: mobile.site when trying to use Minerva on desktop
Closed, ResolvedPublic1 Story Points

Description

When visiting the desktop version of Minerva via https://en.wikipedia.org/wiki/Main_Page?useskin=minerva @Esanders points out an error is thrown in the console.

The error is:

load.php?debug=false&lang=en&modules=jquery%2Cmediawiki|mediawiki.legacy.wikibits&only=scripts&skin…:177 Error: Unknown dependency: mobile.site Error: Unknown dependency: mobile.site
    at sortDependencies (load.php?debug=false&lang=en&modules=jquery%2Cmediawiki|mediawiki.legacy.wikibits&only=scripts&skin…:159)
    at resolveStubbornly (load.php?debug=false&lang=en&modules=jquery%2Cmediawiki|mediawiki.legacy.wikibits&only=scripts&skin…:160)
    at Object.load (load.php?debug=false&lang=en&modules=jquery%2Cmediawiki|mediawiki.legacy.wikibits&only=scripts&skin…:171)
    at Main_Page?useskin=minerva:10
    at startUp (load.php?debug=false&lang=en&modules=startup&only=scripts&skin=minerva:79)
    at HTMLScriptElement.script.onload.script.onreadystatechange (load.php?debug=false&lang=en&modules=startup&only=scripts&skin=minerva:80)

mobile.site should only load on the mobile site as it is a direct replacement for common.js

Please move this logic:
https://github.com/wikimedia/mediawiki-skins-MinervaNeue/blob/master/includes/skins/SkinMinerva.php#L1316
into the MobileFrontend extension inside the onBeforePageDisplay when operating in mobile mode.

Acceptance criteria

  • mobile.site should load for all skins when useformat=mobile is applied.

e.g. Vector, | Minerva.

  • mobile.site should not load when useformat=desktop is applied. site should load instead.

Details

Related Gerrit Patches:
mediawiki/skins/MinervaNeue : masterMobileFrontend should be responsible for adding mobile.site module
mediawiki/extensions/MobileFrontend : masterLoad mobile.site on all mobile skins

Event Timeline

Should mobile.site load for all skins and all MF pages, or just Minerva?

Jdlrobson updated the task description. (Show Details)Jul 19 2017, 4:02 PM

Thanks. I commented out mobile.site to test, and the next exception thrown is on this line in skins.minerva.editor/init.js

if ( mw.config.get( 'wgIsMainPage' ) || isNewPage || page.getLeadSectionElement().text() ) {

Where getLeadSectionElement returns null.

That's a different bug tracked in T157995

Yes I used to subscribe to those groups. But now I just do a (w)diff of
RELEASE-NOTES. Alas the mobile stuff has no RELEASE-NOTES like files.
Please add them.

Also next time be sure to post in
https://lists.wikimedia.org/mailman/listinfo/mediawiki-announce

The error message just looks like a common error message. Like the
administrator did something wrong.

The error message should say

  • BREAKING CHANGE ***

Please tell your wiki Administrator, we have changed ...
so you should change ...
to get your wiki back working. (No this is not a ransom message.)

  • BREAKING CHANGE ***

Had you not had very visible exceptions, it would have been assumed you redesigned your mobile site.

Huh?
If it looked the same, then nobody would think I redesigned the site.
If it looked different, then people would think I redesigned the site, no?

Anyway I'm not sure, but I think now that I added
$wgMFDefaultSkinClass = "SkinVector";
my site thankfully looks the same.

I recall
$wgMFDefaultSkinClass = "SkinMinerva";
was the default for Wikipedia, but I'm not Wikipedia.

Although I always like to stay on the recommended track and would
like to upgrade to MinervaNeue however I couldn't even get the editor to
start, T172643 , so maybe I'll wait till all the bugs are worked out.

Yes after each installation I now should check

  • a random desktop page
  • a random mobile page
  • editing in desktop
  • editing in mobile.

I was only doing the first.

Jdlrobson raised the priority of this task from Normal to High.EditedAug 8 2017, 5:40 PM

Huh?
If it looked the same, then nobody would think I redesigned the site.
If it looked different, then people would think I redesigned the site, no?

It would look different if you used Minerva as your skin e.g. https://en.m.wikipedia.org would look like https://en.m.wikipedia.org?useskin=vector

$wgMFDefaultSkinClass = "SkinVector";
my site thankfully looks the same.

I'm not sure I follow. Are you saying your mobile site uses (and has always used) the Vector skin? If so, I have to ask why you were using MobileFrontend before this change. Doesn't sound like you need it.

http://radioscanningtw.jidanni.org/index.php?useformat=mobile (your mobile site) looks exactly the same as http://radioscanningtw.jidanni.org/index.php?useformat=desktop to me. Is this intentional? If so you don't need MobileFrontend...

On the other hand, the Minerva skin looks like it's working perfectly fine to me:
http://radioscanningtw.jidanni.org/index.php?useformat=mobile&useskin=minerva
This particular bug only impacts the desktop version of the skin which you are not using.

Jidanni added a comment.EditedAug 8 2017, 6:35 PM

I recall I was using MobileFrontend because way in the past that was the only way to accommodate mobile users. Now that apparently vector can handle mobile too, OK I'll try removing MobileFrontend..

http://radioscanningtw.jidanni.org/index.php?useformat=mobile&useskin=minerva looks much better. OK I'll turn that back on once the editor works.

it's worth taking a read through the updated https://www.mediawiki.org/wiki/Extension:MobileFrontend - if you want a different skin for mobile, then you probable do want MobileFrontend. Edit button seems to work fine for me on http://radioscanningtw.jidanni.org/index.php?useformat=mobile&useskin=minerva . I have commented on the relevant ticket.

I am sorry. I think I have three bugs here. I think some of them are merged. When I post a reply it seems to end up getting posted on a different one of these bugs. Maybe I often paste to the wrong browser window. Forgive me.

Yes I want to do what Wikipedia does, as that looks good. So I'll put back Minev... skin soon.

Change 371498 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] MobileFrontend should be responsible for adding mobile.site module

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

Change 371499 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Load mobile.site on all mobile skins

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

Fix is trivial and it's bothered a few people now so I'd appreciate a review of https://gerrit.wikimedia.org/r/371499 if anyone has some spare time!

Jdlrobson set the point value for this task to 1.

Change 371499 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Load mobile.site on all mobile skins

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

Change 371498 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] MobileFrontend should be responsible for adding mobile.site module

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

Jdlrobson closed this task as Resolved.Aug 11 2017, 6:05 PM
Jdlrobson claimed this task.
Jdlrobson removed a project: Patch-For-Review.