Page MenuHomePhabricator

After selecting table cell by single clicking, typing with any IME does not work
Closed, ResolvedPublic8 Story Points

Description

In order to reproduce just select any cell in any table in VisualEditor and start typing within IME. Input should appear inside the cell but instead it does not appear at all.

I investigated it a little bit: normally (not IME) when user starts typing method handleInsertion is called (via keypress handler) and that method change table cell selection to text selection but for IME input it does not happen (because there is no keypress).

Event Timeline

Inez created this task.Jan 12 2015, 9:55 PM
Inez raised the priority of this task from to Needs Triage.
Inez updated the task description. (Show Details)
Inez added a project: VisualEditor.
Inez added a subscriber: Inez.
Inez set Security to None.Jan 12 2015, 10:00 PM
Inez added subscribers: Esanders, dchan.
Jdforrester-WMF renamed this task from After selecting table cell by single clicking typing with IME (any IME) does not work to After selecting table cell by single clicking, typing with any IME does not work.Jan 13 2015, 1:52 AM
Jdforrester-WMF triaged this task as High priority.
Jdforrester-WMF moved this task from To Triage to Freezer on the VisualEditor board.
Jdforrester-WMF lowered the priority of this task from High to Normal.Jan 15 2015, 12:12 AM
Inez added a comment.Mar 3 2015, 11:28 PM

This is a quick and dirty solution: http://pastebin.com/PQr3uZ5S. At this moment it copies code from handleInsertion but it could be just extracted to a method.
@dchan, @Esanders - does it seem like a right solution?

dchan added a comment.Nov 30 2015, 8:00 AM

Inez, now that we've fixed some timing issues I think this solution may be safe - I'll experiment.

dchan added a comment.Nov 30 2015, 3:58 PM

Hah, contrary to my prediction, with this change I can consistently *segfault* Firefox on Ubuntu by clicking a table cell and typing a backtick (which in the UK extended keyboard layout is produced by pressing a deadkey twice, the second press of which causes a compositionstart event). I'll investigate further but I thought this was too good not to share :-)

dchan added a comment.Dec 2 2015, 11:50 AM

The crashing behaviour seems similar to this minimal test case http://jsfiddle.net/s1ryfmp5/ .

That jsfiddle uses an alert, but something in our code is producing the same effect. It seems IME-sensitive: it's crashing for me on Ubuntu 15.10 but not for @Esanders on Ubuntu 14.04 . Continuing to investigate (and will fire upstream bugs as appropriate when done).

dchan added a comment.Dec 2 2015, 6:33 PM

It seems there's a problem specific to IME listeners in Firefox:

  1. An alert() in a compositionstart listener triggers an immediate compositionend (and then segfaults when the dialog is closed).
  2. An alert() in a compositionend listener unblocks keyboard events (including those caused by subsequent keystrokes).

I still don't know what VE is doing that's analogous to the minimal test case's alert() calls, but I suspect it's to do with changing the selection.

dchan added a comment.Dec 4 2015, 11:32 AM

The problem as it affects VE amounts to doing blur();focus() in a compositionstart listener. See https://bugzilla.mozilla.org/show_bug.cgi?id=1230473 and http://jsfiddle.net/6tsyptoh/2/ .

dchan added a comment.Dec 4 2015, 12:42 PM

See also https://bugzilla.mozilla.org/show_bug.cgi?id=1230520 and http://jsfiddle.net/rkprkmbs/1/ , which show further issues with compositionend listeners that might bite us here if the segfault bug is fixed.

Change 264273 had a related patch set uploaded (by Divec):
Delete cross-branchNode selections etc. on compositionstart

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

Change 264273 has a fix for this bug, but with the fix disabled for Firefox because of https://bugzilla.mozilla.org/show_bug.cgi?id=1230473 .

Change 264273 merged by jenkins-bot:
Delete cross-branchNode selections etc. on compositionstart

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

Jdforrester-WMF closed this task as Resolved.Jan 19 2016, 1:02 AM
Jdforrester-WMF assigned this task to dchan.
Jdforrester-WMF edited a custom field.
Jdforrester-WMF moved this task from Freezer to TR3: Language support on the VisualEditor board.