Page MenuHomePhabricator
Create Task
Maniphest T198765

Move mobile editor code from Minerva skin to MobileFrontend, use it for all skins on mobile (and none on desktop)
Closed, ResolvedPublic
Actions

Description

As discussed on T197834:

In T197834#4305356, @matmarex wrote:

The mobile wikitext editor is always used on Minerva skin, in both mobile and desktop mode, even though the desktop editor is much more useful on desktop. (…)

The desktop editor is always used on all other skins, even in mobile mode, even though it seems to me that the mobile editor would work fine with other skins, (…).

Can we instead change this behavior, so that the mobile editor is used on mobile regardless of skin, and the desktop editor is used on desktop regardless of skin?

Developer notes

The following code is the minimum needed to get editor showing on the Timeless mobile skin with some obvious challenges to work out:

mw.config.set('wgMinervaMenuData', {})
mw.loader.using('skins.minerva.editor').then(function() {
  mw.mobileFrontend.require('mobile.startup/Overlay').prototype.appendToElement = 'body'
  window.location.hash = '#/editor/0';
} )

Goals

  1. Stop loading the mobile editor on desktop Minerva (✔ done T198765#4698861)
  2. Move the code from Minerva to MobileFrontend (✔ done T198765#4744389)
  3. Allow loading the mobile editor on other mobile skins (❌ see T198765#4744472)

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
Remove 'skins.minerva.editor' module (moved to MobileFrontend)mediawiki/skins/MinervaNeuemaster+1 -338
Add 'mobile.editor' module (moved from Minerva)mediawiki/extensions/MobileFrontendmaster+361 -1
mobile.startup/Overlay: Compatibility with skins other than Minervamediawiki/extensions/MobileFrontendmaster+10 -7
skins.minerva.editor: Remove unused dependenciesmediawiki/skins/MinervaNeuemaster+8 -8
skins.minerva.scripts/init: Use singleton OverlayManager from MFmediawiki/skins/MinervaNeuemaster+1 -1
skins.minerva.editor: Remove remaining uses of 'skins.minerva.scripts'mediawiki/skins/MinervaNeuemaster+3 -2
mobile.startup/OverlayManager: Add #getSingletonmediawiki/extensions/MobileFrontendmaster+30 -6
Merge remote-tracking branch 'gerrit/page-issues-cleanup'mediawiki/skins/MinervaNeuemaster+0 -114
ResourceLoaderStartUpModule: Add 'wgReadOnly'mediawiki/coremaster+4 -0
Move pageIssues code out of skins.minerva.editormediawiki/skins/MinervaNeuemaster+9 -10
Disable mobile editor (skins.minerva.editor) on desktopmediawiki/skins/MinervaNeuemaster+4 -1
skins.minerva.newusers: Remove dependency on skins.minerva.editormediawiki/skins/MinervaNeuemaster+2 -1
Move edit link enabling/disabling out of skins.minerva.editormediawiki/skins/MinervaNeuemaster+45 -44
Make Minerva section editing more like other skinsmediawiki/skins/MinervaNeuemaster+32 -64
Move redlink CTA out of skins.minerva.editormediawiki/skins/MinervaNeuemaster+40 -27
Remove fancy block info popups (move to MobileFrontend)mediawiki/skins/MinervaNeuemaster+11 -237
Add fancy block info popups from Minerva to MobileFrontendmediawiki/extensions/MobileFrontendmaster+305 -59
Show related patches Customize query in gerrit

Related Objects
Search...

StatusSubtypeAssignedTask
 OpenNoneT158181 Aim for workflow equivalence for MediaWiki on desktop and mobile web
 DeclinedNoneT144571 Mobile Media Viewer violates user settings
 OpenJdlrobsonT65504 Consolidate code for MediaViewer across desktop and mobile
 DeclinedNoneT162210 [EPIC] Upstream MobileFrontend components to core and other extensions (tracking)
 DuplicateJdlrobsonT66316 [EPIC] Move Nearby code to more suitable extension
 OpenNoneT49145 Formally deprecate jQuery UI after we've stopped using jQuery UI in extensions and core
 OpenNoneT100270 Replace use of jQuery UI and MW UI with OOUI across all Wikimedia-deployed extensions and core
 DeclinedNoneT96202 [EPIC] Rewrite MobileFrontend frontend code in OOUI
 DuplicateNoneT101462 [EPIC] Merge OOJS UI library and Mobile unnamed library.
 OpenNoneT96186 [EPIC] Move VisualEditor code from MobileFrontend to VisualEditor
 OpenNoneT106267 mobile phone wikitext editor should have edit toolbar
 OpenNoneT106266 Provide a "signature" button in the mobile wikitext editor
 OpenNoneT171974 Improve and consolidate our unified editing platform so that it's great on all devices
 OpenNoneT159971 Provide the VE wikitext editing mode as the mobile wikitext editor
 ResolvedmatmarexT198765 Move mobile editor code from Minerva skin to MobileFrontend, use it for all skins on mobile (and none on desktop)
 ResolvedmatmarexT202978 Move MobileFrontend's VisualEditor styles back to MobileFrontend
 ResolvedovasilevaT205382 Remove code related to no-anonymous-editing mode of MobileFrontend ($wgMFEditorOptions['anonymousEditing'] = false;)
 ResolvedABorbaWMFT208514 Merge PageIssues Feature Branch to Master
 ResolvedNoneT206178 Feature branch: Remove pageIssues A-B test code

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
• marcella added a project: VisualEditor.Sep 19 2018, 1:45 PM
gerritbot added a comment.Sep 21 2018, 4:57 AM

Change 461852 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/skins/MinervaNeue@master] Move redlink CTA out of skins.minerva.editor

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

gerritbot added a project: Patch-For-Review.Sep 21 2018, 4:57 AM

Change 461853 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/skins/MinervaNeue@master] Move edit link enabling/disabling out of skins.minerva.editor

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

gerritbot added a comment.Sep 21 2018, 4:57 AM

Change 461854 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/skins/MinervaNeue@master] Move pageIssues code out of skins.minerva.editor

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

gerritbot added a comment.Sep 21 2018, 4:57 AM

Change 461855 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/skins/MinervaNeue@master] Make Minerva section editing more like other skins

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

gerritbot added a comment.Sep 21 2018, 4:57 AM

Change 461856 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/skins/MinervaNeue@master] skins.minerva.newusers: Remove dependency on skins.minerva.editor

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

matmarex added a comment.EditedSep 21 2018, 4:59 AM
In T198765#4396195, @matmarex wrote:

I propose we do this in three steps (separate patches for each, and we can stop after step 1 or 2 if we think that's better):

  1. Move the code from Minerva to MobileFrontend, stop loading the mobile editor on desktop, but keep it "whitelisted" to Minerva

I spent the day today working on this, and I think step 1 is ~80% done. I ended up producing a dozen smaller patches rather than a single big one, hopefully that makes review easier. (There are also a few cleanup patches I didn't tag against this task, submitted as dependencies in Gerrit.)

All relevant patches: https://gerrit.wikimedia.org/r/q/topic:T198765

I think only the following remains:

  • Move 'wgMinervaReadOnly' mw.config option to MediaWiki core
  • Move the singleton 'skins.minerva.scripts/overlayManager' to MobileFrontend (the class 'mobile.startup/OverlayManager' is already there)
  • Move the singleton 'skins.minerva.scripts/skin' to MobileFrontend (it's actually an alias for 'mobile.init/skin' defined there)
  • Figure out why the module 'skins.minerva.editor' depends on 'skins.minerva.icons.images.scripts', 'skins.minerva.scripts', and 'skins.minerva.toggling', since it doesn't seem to use any of them, and hopefully remove these dependencies
  • Move the module 'skins.minerva.editor' itself to MobileFrontend!
matmarex edited projects, added VisualEditor (Current work); removed VisualEditor.Sep 21 2018, 5:01 AM
matmarex moved this task from Incoming to Code review on the VisualEditor (Current work) board.
• marcella awarded a token.Sep 21 2018, 12:58 PM
Jdlrobson added a comment.Sep 26 2018, 8:32 PM

Move 'wgMinervaReadOnly' mw.config option to MediaWiki core

🎆

Move the singleton 'skins.minerva.scripts/overlayManager' to MobileFrontend (the class 'mobile.startup/OverlayManager' is already there)
Move the singleton 'skins.minerva.scripts/skin' to MobileFrontend (it's actually an alias for 'mobile.init/skin' defined there)

I'm not sure about these moves. It feels too risky to me as it requires changing all the require paths.
For skin, I'd advise exploring updating the editor to make use of 'mobile.init/skin'.
For OverlayManager - I'd advise rewiring the code so that OverlayManager has a getSingleton method that caches an instance of OverlayManager. Continue to export 'skins.minerva.scripts/overlayManager' but tie it to that singleton method instead.

Figure out why the module 'skins.minerva.editor' depends on 'skins.minerva.icons.images.scripts', 'skins.minerva.scripts', and 'skins.minerva.toggling', since it doesn't seem to use any of them, and hopefully remove these dependencies

Seems a bit weird to me and probably incorrect. Looks like these dependencies exist before 2cc9516c (so old).
skins.minerva.scripts is likely a required dependency because of all the scaffolding that provides - skin and overlayManager. skins.minerva.toggling and skins.minerva.icons.images.scripts' seems unnecessary and can be removed (pending some testing!)

• Deskana closed subtask T202978: Move MobileFrontend's VisualEditor styles back to MobileFrontend as Resolved.Sep 27 2018, 1:30 PM
matmarex mentioned this in T205658: Editing caret is off by several pixels in VisualEditor wikitext editor..Sep 27 2018, 7:59 PM
gerritbot added a comment.Sep 27 2018, 8:16 PM

Change 461854 abandoned by Bartosz Dziewoński:
Move pageIssues code out of skins.minerva.editor

Reason:
OK, let's skip this change due to the ongoing A/B test (looks like T200792 is the relevant task). Hopefully this code will be just removed after it ends.

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

gerritbot added a comment.Sep 27 2018, 10:07 PM

Change 461852 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Move redlink CTA out of skins.minerva.editor

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

ReleaseTaggerBot edited projects, added MW-1.32-notes (WMF-deploy-2018-10-02 (1.32.0-wmf.24)); removed MW-1.32-notes (WMF-deploy-2018-07-31 (1.32.0-wmf.15)).Sep 27 2018, 11:00 PM
matmarex mentioned this in T159971: Provide the VE wikitext editing mode as the mobile wikitext editor.Oct 2 2018, 4:51 PM
matmarex added a parent task: T159971: Provide the VE wikitext editing mode as the mobile wikitext editor.
gerritbot added a comment.Oct 4 2018, 9:22 PM

Change 461853 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Move edit link enabling/disabling out of skins.minerva.editor

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

gerritbot added a comment.Oct 4 2018, 9:26 PM

Change 461855 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Make Minerva section editing more like other skins

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

ReleaseTaggerBot edited projects, added MW-1.32-notes (WMF-deploy-2018-10-16 (1.32.0-wmf.26)); removed MW-1.32-notes (WMF-deploy-2018-10-02 (1.32.0-wmf.24)).Oct 4 2018, 10:01 PM
gerritbot added a comment.Oct 10 2018, 10:04 PM

Change 461856 abandoned by Bartosz Dziewoński:
skins.minerva.newusers: Remove dependency on skins.minerva.editor

Reason:
Looks like we want to remove this code instead (T205382).

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

matmarex added a subtask: T205382: Remove code related to no-anonymous-editing mode of MobileFrontend ($wgMFEditorOptions['anonymousEditing'] = false;).Oct 12 2018, 3:37 PM
Esanders subscribed.Oct 12 2018, 4:05 PM

All the patches here have been merged. Can we move this on from CodeReview?

matmarex moved this task from Code review to In progress on the VisualEditor (Current work) board.Oct 12 2018, 6:16 PM
matmarex removed a project: Patch-For-Review.
ovasileva closed subtask T205382: Remove code related to no-anonymous-editing mode of MobileFrontend ($wgMFEditorOptions['anonymousEditing'] = false;) as Resolved.Oct 24 2018, 5:20 PM
matmarex changed the task status from Open to Stalled.Oct 26 2018, 6:50 PM
matmarex added a subtask: T200792: [EPIC] Run A/B test on page issues (Farsi, Japanese, Russian, English).
matmarex mentioned this in T208068: Can't open the visual editor on MinervaNeue on desktop.Oct 26 2018, 7:17 PM
gerritbot added a comment.Oct 26 2018, 9:43 PM

Change 470087 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/skins/MinervaNeue@master] Disable mobile editor (skins.minerva.editor) on desktop

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

gerritbot added a project: Patch-For-Review.Oct 26 2018, 9:43 PM
matmarex added a subtask: T208102: Improve compatibility of desktop VisualEditor with desktop Minerva skin.Oct 26 2018, 10:21 PM
Jdforrester-WMF moved this task from In progress to Code review on the VisualEditor (Current work) board.Oct 30 2018, 3:33 PM
gerritbot added a comment.Nov 5 2018, 10:11 PM

Change 470087 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Disable mobile editor (skins.minerva.editor) on desktop

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

gerritbot added a comment.Nov 6 2018, 7:13 PM

Change 461854 restored by Bartosz Dziewoński:
Move pageIssues code out of skins.minerva.editor

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

gerritbot added a comment.Nov 6 2018, 7:19 PM

Change 461854 abandoned by Bartosz Dziewoński:
Move pageIssues code out of skins.minerva.editor

Reason:
Superseded by https://gerrit.wikimedia.org/r/c/mediawiki/skins/MinervaNeue/ /467842 (on "page-issues-cleanup" branch, to be merged into master soon T208514).

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

matmarex edited subtasks, added: T208514: Merge PageIssues Feature Branch to Master, T206178: Feature branch: Remove pageIssues A-B test code; removed: T200792: [EPIC] Run A/B test on page issues (Farsi, Japanese, Russian, English).Nov 6 2018, 7:20 PM
gerritbot added a comment.Nov 6 2018, 7:41 PM

Change 472038 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] ResourceLoaderStartUpModule: Add 'wgReadOnly'

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

matmarex moved this task from Code review to In progress on the VisualEditor (Current work) board.Nov 7 2018, 7:05 PM
gerritbot added a comment.Nov 8 2018, 3:33 PM

Change 472477 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/MobileFrontend@master] mobile.startup/OverlayManager: Add #getSingleton

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

gerritbot added a comment.Nov 8 2018, 3:34 PM

Change 472478 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/skins/MinervaNeue@master] Merge remote-tracking branch 'gerrit/page-issues-cleanup'

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

gerritbot added a comment.Nov 8 2018, 3:34 PM

Change 472479 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/skins/MinervaNeue@master] skins.minerva.scripts/init: Use singleton OverlayManager from MF

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

gerritbot added a comment.Nov 8 2018, 3:34 PM

Change 472480 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/skins/MinervaNeue@master] skins.minerva.editor: Remove remaining uses of 'skins.minerva.scripts'

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

matmarex added a comment.Nov 8 2018, 3:37 PM
In T198765#4620518, @Jdlrobson wrote:

Move 'wgMinervaReadOnly' mw.config option to MediaWiki core

🎆

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

Krinkle has objected to adding new mw.config options. Meh, I guess we can just leave it in Minerva only. In the worst case, the editor will not see the read-only warning before beginning their edit, only when attempting to save it.

Move the singleton 'skins.minerva.scripts/overlayManager' to MobileFrontend (the class 'mobile.startup/OverlayManager' is already there)
Move the singleton 'skins.minerva.scripts/skin' to MobileFrontend (it's actually an alias for 'mobile.init/skin' defined there)

I'm not sure about these moves. It feels too risky to me as it requires changing all the require paths.
For skin, I'd advise exploring updating the editor to make use of 'mobile.init/skin'.
For OverlayManager - I'd advise rewiring the code so that OverlayManager has a getSingleton method that caches an instance of OverlayManager. Continue to export 'skins.minerva.scripts/overlayManager' but tie it to that singleton method instead.

https://gerrit.wikimedia.org/r/472477
https://gerrit.wikimedia.org/r/472479
https://gerrit.wikimedia.org/r/472480

matmarex mentioned this in T208514: Merge PageIssues Feature Branch to Master.Nov 8 2018, 3:38 PM
Jdlrobson added a comment.Nov 8 2018, 3:41 PM

Krinkle has objected to adding new mw.config options.

Yeh I saw that. FWIW I think many of the default configuration options are not useful on Minerva... and I feel this one is! Maybe in future we can explore pushing for this again when the current config state is in a cleaner state (see T186062 for example).

matmarex added a comment.Nov 8 2018, 4:03 PM

I am not sure if wgMinervaReadOnly/wgReadOnly is that useful anyway. Currently we use it to prevent opening the editor, but…

I would expect that read-only mode usually lasts for at most a few minutes (when it is triggered by database lag or something). So it is likely that a wiki that was read-only when the view mode page was opened is no longer read-only when the user tries to open the edit mode.

Instead of using a mw.config option, I think it would be better to query for read-only mode with the API request that loads the page wikitext, and treat it as a warning instead of an error. But since this isn't as trivial as just moving/renaming the option, we should probably discuss this in a separate task.

gerritbot added a comment.Nov 8 2018, 4:04 PM

Change 472038 abandoned by Bartosz Dziewoński:
ResourceLoaderStartUpModule: Add 'wgReadOnly'

Reason:
Possibly this isn't a great idea anyway (see T198765#4732369). But regardless, it can stay in Minerva for now.

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

gerritbot added a comment.Nov 8 2018, 5:57 PM

Change 472504 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/MobileFrontend@master] mobile.startup/Overlay: Compatibility with skins other than Minerva

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

gerritbot added a comment.Nov 8 2018, 5:58 PM

Change 472505 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/skins/MinervaNeue@master] skins.minerva.editor: Remove unused dependencies

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

matmarex added a comment.Nov 8 2018, 6:00 PM

After all the new patches above are merged, I think we should be able to move 'skins.minerva.editor' to MobileFrontend without creating any weird dependencies on Minerva :D

Running mw.loader.load( 'skins.minerva.editor' ) will now load the mobile editor on any skin in mobile mode, and it will actually work and be able to edit pages!

Many things still look obviously broken when using it (a bunch of styles for the toolbar and buttons seem missing), but it kind of works, and the rest should be easy to track down (if perhaps tedious).

matmarex added a comment.Nov 8 2018, 6:10 PM

Here's how the mobile editor looks like on Vector right now ;)

image.png (640×360 px, 33 KB)

matmarex changed the task status from Stalled to Open.Nov 8 2018, 6:11 PM
matmarex moved this task from In progress to Code review on the VisualEditor (Current work) board.
Jdlrobson added a comment.Nov 8 2018, 6:18 PM

Please move this code to the "mobile.init" module when making that migration. That's designed to load on all pages running in mobile mode.

gerritbot added a comment.Nov 9 2018, 1:11 AM

Change 472478 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Merge remote-tracking branch 'gerrit/page-issues-cleanup'

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

gerritbot added a comment.Nov 9 2018, 5:19 PM

Change 472477 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] mobile.startup/OverlayManager: Add #getSingleton

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

gerritbot added a comment.Nov 9 2018, 5:19 PM

Change 472479 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] skins.minerva.scripts/init: Use singleton OverlayManager from MF

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

gerritbot added a comment.Nov 9 2018, 5:19 PM

Change 472480 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] skins.minerva.editor: Remove remaining uses of 'skins.minerva.scripts'

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

ReleaseTaggerBot added a project: MW-1.33-notes (1.33.0-wmf.4; 2018-11-13).Nov 9 2018, 6:00 PM
gerritbot added a comment.Nov 12 2018, 8:48 PM

Change 472505 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] skins.minerva.editor: Remove unused dependencies

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

Jdlrobson mentioned this in T209129: MFA: Drawers and Overlays should not auto-append it self to the body element.Nov 12 2018, 11:26 PM
matmarex updated the task description. (Show Details)Nov 13 2018, 9:47 PM
matmarex added a comment.Nov 13 2018, 10:05 PM
In T198765#4732913, @Jdlrobson wrote:

Please move this code to the "mobile.init" module when making that migration. That's designed to load on all pages running in mobile mode.

Currently 'skins.minerva.editor' is actually not used on all pages. It is not loaded on special pages and on pages with content models that require a specialized editor (for example, on Wikisource: try editing https://en.m.wikisource.org/wiki/Page%3ATragedies_of_Euripides_(Way_1896)_v2.djvu/238). So I think it needs to remain a separate module.

gerritbot added a comment.Nov 13 2018, 10:27 PM

Change 473288 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/MobileFrontend@master] Add 'mobile.editor' module (moved from Minerva)

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

gerritbot added a comment.Nov 13 2018, 10:27 PM

Change 473289 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/skins/MinervaNeue@master] Remove 'skins.minerva.editor' module (moved to MobileFrontend)

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

gerritbot added a comment.Nov 13 2018, 10:29 PM

Change 472504 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] mobile.startup/Overlay: Compatibility with skins other than Minerva

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

matmarex added a comment.Nov 13 2018, 10:43 PM

Goals

  1. Stop loading the mobile editor on desktop Minerva
  2. Move the code from Minerva to MobileFrontend
  3. Allow loading the mobile editor on other mobile skins

I added this to the task description just now, but it describes what I've been trying to do here from the start. But after some consideration, I'm going to remove #3 from the scope of this task.

I've done some work towards this (mostly in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MobileFrontend/+/472504 and https://gerrit.wikimedia.org/r/c/mediawiki/skins/MinervaNeue/+/461855) and at this point, it is possible to use mw.loader.load( 'skins.minerva.editor' ) (or mw.loader.load( 'mobile.editor' )) 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.

Jdlrobson awarded a token.Nov 13 2018, 10:44 PM
Jdlrobson added a subscriber: Isarra.

Thanks! This is looking way cleaner! Maybe @Isarra can help us move this forward by consuming these in Timeless and giving us feedback on how they could work better?

matmarex updated the task description. (Show Details)Nov 13 2018, 10:45 PM
ReleaseTaggerBot edited projects, added MW-1.33-notes (1.33.0-wmf.6; 2018-11-27); removed MW-1.33-notes (1.33.0-wmf.4; 2018-11-13).Nov 13 2018, 11:00 PM
Isarra added a comment.Nov 14 2018, 1:28 AM

I'm not sure what you mean here by consuming. Does the skin needs to explicitly load it or something for it to activate? Or is this more of a VE-like deal where we just need to make sure the structure is actually, uh, what we expect?

gerritbot added a comment.Nov 14 2018, 11:37 PM

Change 473288 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Add 'mobile.editor' module (moved from Minerva)

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

gerritbot added a comment.Nov 14 2018, 11:37 PM

Change 473289 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Remove 'skins.minerva.editor' module (moved to MobileFrontend)

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

matmarex moved this task from Code review to Engineering QA on the VisualEditor (Current work) board.Nov 14 2018, 11:59 PM
matmarex removed a project: Patch-For-Review.

This is done from my perspective. (The subtasks are also done, waiting on QA.) Thanks for all the reviews, @Jdlrobson!

@Esanders Can you take a look at the end result to see if it seems sane?

Jdlrobson closed subtask T208514: Merge PageIssues Feature Branch to Master as Resolved.Nov 15 2018, 4:51 PM
matmarex mentioned this in T186735: VisualEditor toolbar overlaps with desktop Minerva header.Nov 20 2018, 5:59 AM
gerritbot added a comment.Nov 21 2018, 8:34 PM

Change 475147 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] View's now pass isBorderBox as a property

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

gerritbot added a project: Patch-For-Review.Nov 21 2018, 8:34 PM
matmarex removed a project: Patch-For-Review.Nov 21 2018, 8:58 PM

(Unrelated patch, it is actually for T209007)

Esanders moved this task from Engineering QA to Product owner review on the VisualEditor (Current work) board.Jan 17 2019, 2:46 PM
ppelberg closed this task as Resolved.Mar 13 2019, 6:57 PM
matmarex mentioned this in T190989: On the desktop site, editing the lead section of a page under MinervaNeue skin directs a user to one of desktop WikiEditors.Mar 26 2019, 8:36 PM
matmarex removed a subtask: T208102: Improve compatibility of desktop VisualEditor with desktop Minerva skin.May 1 2019, 8:56 PM
matmarex mentioned this in T257746: VisualEditor is not initialized on any other mobile skin than Minerva Neue.Jul 14 2020, 7:48 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits