Page MenuHomePhabricator

Give users the ability to choose skin for mobile domain in Special:Preferences
Open, LowestPublic

Description

Looking at the decoupling of MobileFrontend and Minerva, it would be nice if it is possible to explicitly choose a skin for mobile view and desktop view. Standard setting would be Vector for skin and MinervaNeue for mobileskin.

While this might be not that useful in the current situation, it gains practical use if more skins are added.

Developer notes

  • A patch for MobileFrontend exists
  • Once merged, any site needing the feature can enable this feature via LocalSettings.php edit.

Event Timeline

Jdlrobson triaged this task as Lowest priority.Aug 17 2017, 9:11 PM
Jdlrobson edited projects, added MinervaNeue (3rd party support); removed MinervaNeue.
Jdlrobson subscribed.

You can do this currently via hidden user preference mobileskin.

We purposely don't expose this to avoid the headaches of having to actively maintain this and to avoid promoting bad mobile skins (on Wikimedia wikis we currently do not have any alternative mobile friendly skins so this is not a priority).

A new extension or a localsettings hook could easily expose the hidden preference in the mean time if it wanted to.

To be honest, I use Vector in mobile view, because Minerva isn't practical for editors in my opinion: Watchlist is nearly unusable, you can't scan through talk pages, etc. Switching skins with the link at the bottom of the page can be a pain because of the lazy loading of the watchlist.

Besides, I filed this requuest with Timeless in mind, that could be an usable mobile skin in some time.

Sure, but the same Watchlist applies to all mobile skins including Timeless and Vector so this will not help you:
https://en.wikipedia.org/wiki/Special:Watchlist?useskin=vector&useformat=mobile
https://mediawiki.org/wiki/Special:Watchlist?useskin=timeless&useformat=mobile

The desktop link at the bottom in the footer is what you want. That will force the desktop skin for mobile views.

The root of the problem is the HTML for Watchlist and various other pages are not mobile friendly at all. T99256 will probably help with that somewhat.

Sure, but the same Watchlist applies to all mobile skins including Timeless and Vector so this will not help you:
https://en.wikipedia.org/wiki/Special:Watchlist?useskin=vector&useformat=mobile
https://mediawiki.org/wiki/Special:Watchlist?useskin=timeless&useformat=mobile

Oh, I wasn't aware of that. Actually, has there ever been feedback about this watchlist design? I don't think it's favorable for anyone with more than 100 watched pages, which applies to the majority of mediawiki power users.

The desktop link at the bottom in the footer is what you want. That will force the desktop skin for mobile views.

Yes, but yu have to reach it first.

Change 442221 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Allow users to change their mobile skin preference

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

Change 442221 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Allow users to change their mobile skin preference

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

Change 451045 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Revert "Allow users to change their mobile skin preference"

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

@Jdlrobson, @Jdrewniak noticed a regression where the advanced settings were always shown when this patch was merged. Sorry, but we had to do a quick revert for now.

@Jdlrobson - wondering what the motivation for the timing was and whether this was under a feature flag? I thought we decided in T186760#4305904 that we weren't going to look at this for now?

After digging into the code I found whats wrong:

https://gerrit.wikimedia.org/r/#/c/442221/ breaks the Minerva Skin initialization. The problem is in MobileFrontend.hooks::onRequestContextCreateSkin, code tries to load the userskin, and because we started to store the selected skin in the user options store, code enters the if ( $userSkin )` block (https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/blob/f13e4e98e0261d9ee9780d484803e5610c83836f/includes/MobileFrontend.hooks.php#L13).
Skin is vs valid and because of that code exists early without calling the RequestContextCreateSkinMobile hook.

RequestContextCreateSkinMobile is necessary for the MinervaSkin, Skin listens to that hook to properly initialize itself. Without the hook all skin options (eg: beta mode, toggling, mobile options) defaults to false as defined here.

Change 451045 merged by 20after4:
[mediawiki/extensions/MobileFrontend@master] Revert "Allow users to change their mobile skin preference"

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

It probably shouldn't be possible for changes in MF to break Minerva tests. Should those tests be moved?

@Esanders, the problem was with the Skin initialization system. MobileFrontend changes broke Minerva tests because the MobileFrontend stopped executing RequestContextCreateSkinMobile and the Minerva skin wasn't initialized properly.
By "wasn't initialized properly" I mean that the skinOptionsweren't set properly, all values were set to default false instead of proper values.

Now we have to answer a couple questions/things to follow-up:

  • why Minerva has some defaults set to false and then on RequestContextCreateSkinMobile it changes those variables to true. Those values could be initialized properly during skin creation
  • there should be a unit tests, that MobileFrontend has to call RequestContextCreateSkinMobile hook, otherwise, Mobile Skin system will be broken
  • MinervaNeueu unit tests do not handle the default skinOptions well (there are no checks for "Preferences" as a user option), we always expect "Options" to be in Menu, but code allows to switch between "Preferences" and "Options", etc

You're right, changes in MF shouldn't break Minerva tests. In this scenario, we have to fix both MobileFrontend (add tests to prevent such issues in the future), and we need to improve Minerva test scenarios to properly handle both cases (Preferences and Options)

Sounds good, I appreciate the MF & minerva and somewhat more closely linked that most skins/extensions.

Change 451807 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Allow users to change their mobile skin preference (take 2)

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

RequestContextCreateSkinMobile is necessary for the MinervaSkin, S

It's only necessary for the beta mode :)

, and because we started to store the selected skin in the user options store,

Yes, so I this was the key problem. The code was written in such a way that I thought it was already accessing the user options store. I trusted it, rather than inspecting it. That was wrong.

It turns out this was silently failing, as the options hadn't been registered via onUserGetDefaultOptions.
e.g. This line is silently returning null right now:
https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/blob/master/includes/MobileFrontend.hooks.php#L128

@Esanders, the problem was with the Skin initialization system. MobileFrontend changes broke Minerva tests because the MobileFrontend stopped executing RequestContextCreateSkinMobile and the Minerva skin wasn't initialized properly.
By "wasn't initialized properly" I mean that the skinOptionsweren't set properly, all values were set to default false instead of proper values.

To be clear, Minerva was initialised fine, but the beta mode was not.

Now we have to answer a couple questions/things to follow-up:

  • why Minerva has some defaults set to false and then on RequestContextCreateSkinMobile it changes those variables to true. Those values could be initialized properly during skin creation

This is how beta mode works. Skin's cannot take options in their constructor, which was why we created ICustomizableSkin (which lego is attempting to remove in another patchset). That's why we don't do this in the constructor. We could of course, make the constructor take an options array that defaults to empty and throw away this code.

  • there should be a unit tests, that MobileFrontend has to call RequestContextCreateSkinMobile hook, otherwise, Mobile Skin system will be broken

I don't think a unit test here would have helped this problem. The CI in Minerva did exactly what it was supposed to do - check the integration with MobileFrontend to make sure mobile is working.

  • MinervaNeueu unit tests do not handle the default skinOptions well (there are no checks for "Preferences" as a user option), we always expect "Options" to be in Menu, but code allows to switch between "Preferences" and "Options", etc

You're right, changes in MF shouldn't break Minerva tests. In this scenario, we have to fix both MobileFrontend (add tests to prevent such issues in the future), and we need to improve Minerva test scenarios to properly handle both cases (Preferences and Options)

Many of the tests in Minerva check integration with MobileFrontend so it's possible and by design that MobileFrontend changes can break Minerva (just like changes in MobileFrontend can break extensions like Thanks and Echo).

I've created https://phabricator.wikimedia.org/T201655 to follow up on the Preferences link.

I've rewritten the existing patch to incorporate these 2 new pieces of information:
https://gerrit.wikimedia.org/r/451807

Jdlrobson renamed this task from Allow people to explicitly choose skins for stationary and mobile to Allow people to explicitly choose skins for desktop domain (stationary) and mobile domain.Nov 1 2018, 11:03 PM
Jdlrobson updated the task description. (Show Details)

Heads up: this has come up in the community wishlist.

https://gerrit.wikimedia.org/r/451807 has been rebased. Patch is feature flagged so makes no product decision on enabling it (although it does allow editors to use the api to set the skin preference which was previously broken)

Jdlrobson renamed this task from Allow people to explicitly choose skins for desktop domain (stationary) and mobile domain to Give 3rd party users the ability to choose skin for mobile domain in Special:Preferences.Nov 23 2018, 7:16 PM
Jdlrobson updated the task description. (Show Details)

Change 451807 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Allow users to change their mobile skin preference (take 2)

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

Jdlrobson renamed this task from Give 3rd party users the ability to choose skin for mobile domain in Special:Preferences to Give users the ability to choose skin for mobile domain in Special:Preferences.May 17 2019, 4:42 PM
Jdlrobson added subscribers: Geertivp, Charlotte.

How can I then automatically force desktop mode on my Android tablet Chrome browser for all web sites?

The above MediaWiki option "mobileskin" is not (yet) available from Special:Preferences ?

I have found on internet the about:debug option UAString, about:useragent, and about:settings but I do not find the dektop setting. I have tried the built-in Android brower, Chrome and Opera.

@Geertivp I made two attempts in https://gerrit.wikimedia.org/r/451807 but both got reverted as I did something majorly wrong which broke production. I've not had time to look into the problem and fix it. You're welcome to take a look. Note when that does eventually get fixed this would apply to your account on mobile as well as tablet (although you could create separate accounts to work around that).

You will have to spoof your browser agent to avoid the redirect in the mean time. I don't know how to do that on your Android tablet. A Chrome extension is your best bet if your browser supports them. There are many in the chrome store for doing exactly this.

Thanks @Jdlrobson. On the desktop Chrome I have found the http://osxdaily.com/2013/01/16/change-user-agent-chrome-safari-firefox/ option to set the User agent string via "top left menu bar developer option". This deeply hidden option would allow me to test the mobile website behaviour on my desktop device. Which would be nice for debugging or simulation purposes.

For Opera on mobile I have found the setting Default user agent "desktop" which is exactly what I want. I needed to restart the browser once to effectively enable this option. So I will use Opera as of now as my standard browser on my tablet... I AM HAPPY.

I did not find (yet) the equivalent (reverse setting) on my tablet Chrome...

Hmm, so the last patch got reverted because it had replaced the "Settings" link in the mobile site with the desktop "Preferences" page. This can be easily solved in my opinion.