Page MenuHomePhabricator

Mobile switching with changes from WTE to VE fails on non-RESTBase setups
Closed, ResolvedPublic

Description

Mobile switching with changes from WTE to VE fails on non-RESTBase setups:

TypeError: Called on non-object
    at Object.oo.getObjectValues
    at VeInitMwMobileArticleTarget.ve.init.mw.ArticleTarget.parseMetadata
    at VeInitMwMobileArticleTarget.ve.init.mw.ArticleTarget.loadSuccess
    ...

Looks like we don't run the query to load the metadata we need when switching with changes.

Details

Related Gerrit Patches:
mediawiki/extensions/MobileFrontend : mastereditor: Respect VisualEditor switching options
mediawiki/extensions/VisualEditor : masterAllow switching from WTE to VE with changes in non-RESTBase mode

Event Timeline

matmarex created this task.May 1 2019, 9:46 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 1 2019, 9:46 PM
matmarex claimed this task.May 1 2019, 9:46 PM

Change 507711 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] ve.init.mw.ArticleTargetLoader: Fix non-RESTBase #requestParsoidData with wikitext

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

Switching from WTE to VE with changes shouldn't be possible on a non-restbase setup. Restbase is required to stash the modified HTML version of the doc. On desktop we tell users they can "discard changes and switch".

Hmm, true… I remember now that it never worked on desktop.

Is it supposed to be disabled on non-RESTBase though, or just something that we never got around to implementing? Since it seems to me that my patch above actually works just fine…

It is supposed to be disabled. I'm not sure what happens without the RESTBase stash, maybe a full doc dirty diff?

Hmm, well, would that be so terrible? I think the benefit of being able to switch editors would be greater than the drawback of dirty diffs. (I feel like that's also a much smaller problem on small or private wikis.)

In fact, today during the triage meeting I found this task: T214542: Unable to keep unsaved changes going from source to VisualEditor mode on a private/non-RESTbase wiki which appears to be a request for non-RESTBase switching.

Also, the various differences in behavior between production (RESTBase) and my testing wikis (non-RESTBase) has been causing me no end of annoyance. I think getting rid of this difference would be good for long-term maintenance.

Let's check with Parsoid if we are making the correct assumptions. @ssastry can you shed some light on exactly what happens if we switch from modified WT to VE without stashing?

Let's check with Parsoid if we are making the correct assumptions. @ssastry can you shed some light on exactly what happens if we switch from modified WT to VE without stashing?

[ I didn't read the phab ticket .. so, contextless response ] Since Parsoid can always convert Parsoid HTML to wikitext, without old html to compare against, you get normalized wikitext => dirty diffs.

Hmm, well, would that be so terrible? I think the benefit of being able to switch editors would be greater than the drawback of dirty diffs. (I feel like that's also a much smaller problem on small or private wikis.)

Perhaps, but I can also imagine expanding a large table from | inline || syntax || mode to having each cell on its own line could be annoying, even on a private wiki. Maybe such a switch should come with a warning.

I'm not convinced it's necessary. A warning for end-users would be difficult to explain. Would you find it acceptable if we add a wfDebugLog() in the API code somewhere?

Esanders added a comment.EditedMay 24 2019, 5:57 PM

We should at least document that if you don't install RESTbase you are opening yourself up to this issue. Maybe it should be a config? Then at least people can turn if back off.

Done, this is now controlled by $wgVisualEditorAllowLossySwitching (defaults to true).

Change 513336 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/MobileFrontend@master] editor: Respect VisualEditor switching options

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

Change 507711 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Allow switching from WTE to VE with changes in non-RESTBase mode

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

Change 513336 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] editor: Respect VisualEditor switching options

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

matmarex moved this task from QA to Engineering QA on the VisualEditor (Current work) board.

It has been working on my local wiki perfectly for the last few days while I was working on and testing various other changes related to editor switching.

ppelberg closed this task as Resolved.Jul 10 2019, 11:54 PM