Page MenuHomePhabricator

VisualEditor is not initialized on any other mobile skin than Minerva Neue
Closed, ResolvedPublic

Description

Problem:
MobileFrontend works with any skins now if configured, but the mobile version of VisualEditor is only supported in Minerva Neue because of a hard-coded condition check. (See https://gerrit.wikimedia.org/g/mediawiki/extensions/MobileFrontend/+/67b269ee7297022405e94c7be142ade4594f2d6d/src/mobile.init/mobile.init.js#108)

Option 1. Introducing a new configuration value that is similar to $wgVisualEditorSupportedSkins. (Removed in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/VisualEditor/+/345207)

Option 2. Removing the skin check.

Event Timeline

Restricted Application added subscribers: Masumrezarock100, Aklapper. · View Herald Transcript
Lens0021 renamed this task from VisualEditor is not initialized on any other skin than Minerva Neue to VisualEditor is not initialized on any other mobile skin than Minerva Neue.Jul 12 2020, 10:24 AM
JTannerWMF subscribed.

Considering mobile skins can not be set on Wikimedia wikis mobile sites, we can not prioritize working on this task at this time.

Would there be someone who can review If I make a patch for the option 2 (Simply removing the skin check if statement)?

Would there be someone who can review If I make a patch for the option 2 (Simply removing the skin check if statement)?

I've just tried doing that and the editor indeed initializes, but it's completely unusable – the editor interface is squished in the middle of the screen while the read-mode interface appears behind it, and you can't even scroll the article editing area, among other obvious issues.

Screenshot of mobile editor on Vector, as it would be right now:

image.png (1×828 px, 553 KB)

We did some related work about a year ago nearly 2 years ago… time flies (we wanted to make the desktop VE initialize on Minerva when it is used as a desktop skin – kind of the opposite of your goal, I guess), in T198765. I wrote this at the time, and it still seems to be true now:

I've done some work towards this […] and at this point, it is possible […] to load the mobile editor on any skin, and it mostly works and edits pages. However, it looks horribly broken without a bunch of styles that are only present in Minerva (not even related to the editor itself, but more generally to mobile overlays – I suspect things like the category overlay would also be broken in a similar way).

The missing styles look like a mess and I think someone more familiar with the mobile site's code would be able to fix them up more easily than myself. Main goal here was tech debt cleanup rather than functional changes, so I feel good about this still. I think that right now, allowing other mobile skins is not really a priority for anyone, anyway.

If you're interested in doing all the legwork, I'd probably be interested in reviewing it (and maybe @Jdlrobson too), but unfortunately it's for sure much more work than removing an if.

Maybe I had to mention my motivation first... I working on adding mobile support to a skin that is not deployed by WMF. The skin in front of MobileFrontend is just frozen when the "Edit" button clicked and I thought it couldn't be solved without upstream changes. Although there are many issues that will be solved at WMF, custom skins should have a change of challenges. Please correct me if I'm wrong

Change 783889 had a related patch set uploaded (by Lens0021; author: Lens0021):

[mediawiki/extensions/MobileFrontend@master] Allow skins to enable mobile edit

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

Change 784305 had a related patch set uploaded (by Jdlrobson; author: Lens0021):

[mediawiki/skins/MinervaNeue@master] Set MobileFrontendEditorAvailableSkins attribute

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

Change 783889 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] Allow skins to enable mobile edit

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

Change 784305 merged by jenkins-bot:

[mediawiki/skins/MinervaNeue@master] Set MobileFrontendEditorAvailableSkins attribute

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

The remaining work is to clean up the Minerva specific handling I commented about here:
https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MobileFrontend/+/783889/5/src/mobile.init/mobile.init.js

Should be safe to do that now.

Change 789212 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/MobileFrontend@master] Cleanup minerva check

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

Hi @matmarex can I please request testing/review on the above patch to finish up this task? Not super urgent but would be nice to close out this ticket.

Change 789212 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] Cleanup minerva check

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

@Lens0021 I'd be curious to see your skin in action. Thanks for working on this!