Page MenuHomePhabricator
Create Task
Maniphest T217223

Backspacing multiple VE paragraphs in Gboard causes model corruption
Open, LowPublic
Actions

Description

Steps to reproduce

  1. In Android Chrome using Gboard in English, open the VE standalone h1 demo (e.g. go to https://rawcdn.githack.com/wikimedia/VisualEditor/4669eff2bd4441da2b0c3e19cecafdbc2bcc59b8/demos/ve/mobile.html#!h1 ).
  1. The document should read "abcdefg". Put the cursor after the "d" and press Enter to break the paragraph.
  1. Turn on "Show model" and "Update on change".
  1. Make a selection that crosses the paragraph break (e.g. select "cd" and "ef"). Press backspace.

Expected behaviour: the selected content is removed and the paragraphs are unified.

Actual behaviour: an extra copy of the second paragraph is left behind.

I would guess this is due to the details of the HTML tree changes and DOM events caused by pressing backspace in Gboard in some languages. It happens on mobile Firefox too. It occurs with Gboard English and Welsh, but not with Gboard Cantonese Jyutping. Other input methods seem unaffected.

Details

SubjectRepoBranchLines +/-
Update VE core submodule to master (e2f4b0e96)mediawiki/extensions/VisualEditormaster+1 -1
Revert "Call handleInsertion on beforeinput"VisualEditor/VisualEditormaster+1 -14
Call handleInsertion on beforeinputVisualEditor/VisualEditormaster+14 -1
Update VE core submodule to master (e3715c257)mediawiki/extensions/VisualEditormaster+1 -1
Customize query in gerrit

Related Objects
Search...

Event Timeline

dchan created this task.Feb 27 2019, 4:20 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 27 2019, 4:20 AM
dchan added a comment.Mar 1 2019, 5:12 AM

Ok, with Gboard English, this is what happens when you select "de</h1><h1>fg" and press backspace:

  1. The selection collapses to the end
  2. A keydown event with an unknown keycode is fired
  3. The content is deleted (unifying the paragraphs)

This means our code misunderstands the change as regards the first paragraph, because we make the assumption that keystrokes can only trigger native DOM changes in paragraphs touched by the selection.

The behaviour with Gboard Cantonese Jyutping is different. The backspace key triggers an actual keycode=8 keydown event, and so our code suppresses the event and performs the deletion itself, just as it would on desktop with no IME.

JTannerWMF edited projects, added VisualEditor (Current work); removed VisualEditor.Mar 12 2019, 3:47 PM
matmarex subscribed.Mar 19 2019, 2:09 PM

Depends on T218545: Cope with IMEs that make uncancellable alterations to DOM structural nodes?

gerritbot added a comment.Mar 23 2019, 8:13 AM

Change 498528 had a related patch set uploaded (by Divec; owner: Divec):
[VisualEditor/VisualEditor@master] WIP: Call handleInsertion on beforeinput

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

gerritbot added a project: Patch-For-Review.Mar 23 2019, 8:13 AM
Esanders assigned this task to dchan.Mar 26 2019, 4:28 PM
Esanders moved this task from Incoming to In progress on the VisualEditor (Current work) board.
matmarex unsubscribed.Mar 26 2019, 9:47 PM
dchan added a comment.Apr 4 2019, 11:04 AM

I am reasonably satisfied with Change 498528 now. I cannot find a case where this breaks input, having tested on multiple types of IME, on Chromium, Firefox and Safari, and on Android, iOS, Windows, Linux and MacOS. Of course, there are almost limitless combinations, so it's always possible there is some breakage I've not managed to find.

It remains to be discussed whether we're willing to merge code that depends on beforeinput, which is experimental (currently implemented in webkit browsers, i.e. Chrome and Safari). See https://developer.mozilla.org/en-US/docs/Web/Events/beforeinput .

Note that the change adds beforeinput to the set of events watched by the ve.EventSequencer. In theory this can cause after-event handlers to run slightly earlier. I don't *believe* this has a visible effect in practice, but technically the change is not quite as simple as just changing whether we listen to beforeinput or not.

Esanders subscribed.Apr 4 2019, 11:54 AM

I think we'll just have to try it and keep an eye out for new IME bugs.

Esanders moved this task from In progress to Code review on the VisualEditor (Current work) board.May 24 2019, 8:17 PM
gerritbot added a comment.May 24 2019, 8:19 PM

Change 498528 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] Call handleInsertion on beforeinput

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

Maintenance_bot removed a project: Patch-For-Review.May 28 2019, 3:44 PM
gerritbot added a comment.May 28 2019, 5:46 PM

Change 512938 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (e3715c257)

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

gerritbot added a project: Patch-For-Review.May 28 2019, 5:46 PM
gerritbot added a comment.May 28 2019, 6:16 PM

Change 512938 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (e3715c257)

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

Maintenance_bot removed a project: Patch-For-Review.May 28 2019, 7:10 PM
matmarex moved this task from Code review to Product owner review on the VisualEditor (Current work) board.May 29 2019, 10:25 PM
matmarex added a project: Skipped QA.

I assume this should be fixed by this patch.

Since David already did extensive testing, we should probably skip formal QA.

matmarex added a parent task: T218545: Cope with IMEs that make uncancellable alterations to DOM structural nodes.Jun 8 2019, 7:11 AM
matmarex mentioned this in T221328: [Epic] Mobile VE usability improvements (FY18-19 Q4).Jun 8 2019, 1:55 PM
• ppelberg closed this task as Resolved.Jul 3 2019, 3:12 AM
JTannerWMF mentioned this in T218545: Cope with IMEs that make uncancellable alterations to DOM structural nodes.Jul 3 2019, 12:25 PM
matmarex added a project: VisualEditor-Mobile-UsabilityImprovements.Jul 3 2019, 7:10 PM
gerritbot added a comment.Jul 18 2019, 2:11 PM

Change 523770 had a related patch set uploaded (by Divec; owner: Esanders):
[VisualEditor/VisualEditor@master] Revert "Call handleInsertion on beforeinput"

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

gerritbot added a project: Patch-For-Review.Jul 18 2019, 2:11 PM
gerritbot added a comment.Jul 18 2019, 2:29 PM

Change 523770 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] Revert "Call handleInsertion on beforeinput"

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

Maintenance_bot removed a project: Patch-For-Review.Jul 18 2019, 3:11 PM
gerritbot added a comment.Jul 19 2019, 4:10 PM

Change 524550 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (e2f4b0e96)

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

gerritbot added a project: Patch-For-Review.Jul 19 2019, 4:10 PM
gerritbot added a comment.Jul 19 2019, 4:26 PM

Change 524550 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (e2f4b0e96)

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

matmarex reopened this task as Open.Jul 19 2019, 4:36 PM
matmarex moved this task from Product owner review to Incoming on the VisualEditor (Current work) board.

Reverted, caused T227852.

Maintenance_bot removed a project: Patch-For-Review.Jul 19 2019, 5:11 PM
JTannerWMF moved this task from Incoming to Ready for Development on the VisualEditor (Current work) board.Aug 21 2019, 11:54 AM
JTannerWMF edited projects, added Editing-team (Q3 2019-2020 Kanban Board); removed VisualEditor (Current work).Feb 5 2020, 12:07 AM
JTannerWMF moved this task from Needs Discussion / Investigation to Upcoming on the Editing-team (Q3 2019-2020 Kanban Board) board.
Restricted Application added a project: VisualEditor. · View Herald TranscriptFeb 5 2020, 12:07 AM
JTannerWMF moved this task from Upcoming to Blocked / Needs More Work on the Editing-team (Q3 2019-2020 Kanban Board) board.Feb 5 2020, 6:54 PM
JTannerWMF added subscribers: • ppelberg, JTannerWMF.

@ppelberg and @dchan to determine the priority of this task.

JTannerWMF moved this task from Blocked / Needs More Work to Ready for Development on the Editing-team (Q3 2019-2020 Kanban Board) board.Feb 6 2020, 10:10 PM

After chatting with @dchan we determined he is going to work on this

matmarex moved this task from To Triage to Triaged on the VisualEditor board.Feb 12 2020, 5:24 PM
matmarex triaged this task as Low priority.Feb 20 2020, 6:20 PM
MBinder_WMF edited projects, added Editing-team (Kanban Board); removed Editing-team (Q3 2019-2020 Kanban Board).Jul 9 2020, 4:53 PM
JTannerWMF moved this task from Incoming to Ready to Be Worked On on the Editing-team (Kanban Board) board.Jul 9 2020, 4:53 PM
JTannerWMF edited projects, added Editing-team (Tracking); removed Editing-team (Kanban Board).Jul 9 2020, 8:28 PM
JTannerWMF moved this task from Backlog to FY2020-21 on the Editing-team (Tracking) board.
Aklapper removed dchan as the assignee of this task.Jul 2 2021, 5:24 AM

Removing task assignee due to inactivity, as this open task has been assigned for more than two years (see emails sent to assignee on May26 and Jun17, and T270544). Please assign this task to yourself again if you still realistically [plan to] work on this task - it would be very welcome!

(See https://www.mediawiki.org/wiki/Bug_management/Assignee_cleanup for tips how to best manage your individual work in Phabricator.)

matmarex mentioned this in T302083: Source and preview getting out of sync on Firefox for Android (keyboard / GBoard app may be involved).Feb 24 2022, 5:16 PM
matmarex mentioned this in T312558: Google Keyboard input events can leave VisualEditor DM out of sync.Jul 7 2022, 4:02 PM
gerritbot added a comment.Jul 7 2022, 10:25 PM

Change 812093 had a related patch set uploaded (by TK-999; author: TK-999):

[VisualEditor/VisualEditor@master] [WIP] Resolve some out-of-sync issues on Android with Google Keyboard

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

gerritbot added a project: Patch-For-Review.Jul 7 2022, 10:25 PM
matmarex added a subtask: T312558: Google Keyboard input events can leave VisualEditor DM out of sync.Sep 26 2022, 8:03 PM
• ppelberg closed subtask T312558: Google Keyboard input events can leave VisualEditor DM out of sync as Resolved.Nov 19 2022, 1:16 AM
matmarex mentioned this in T345502: Visual editor unable to remove line break.Sep 4 2023, 6:23 PM
matmarex subscribed.Dec 8 2023, 4:09 PM
Fgnievinski subscribed.Mon, Mar 25, 12:19 AM
Fgnievinski added a comment.Thu, Apr 4, 2:14 AM

I believe this bug makes mobile editing unusable for both articles and talk pages, there's a confirmed report at T345502.

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