Closed
Bug 1083629
Opened 10 years ago
Closed 10 years ago
Composition in a designMode editor is canceled when you click the editor on Linux
Categories
(Core :: DOM: Editor, defect)
Tracking
()
RESOLVED
FIXED
mozilla36
Tracking | Status | |
---|---|---|
firefox34 | --- | unaffected |
firefox35 | + | fixed |
firefox36 | --- | fixed |
People
(Reporter: masayuki, Assigned: masayuki)
References
Details
(Keywords: inputmethod, regression)
Attachments
(1 file)
2.06 KB,
patch
|
ehsan.akhgari
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Looks like this is asynchronous commit issue. Therefore, I guess that this is a regression of bug 1065835 and you can reproduce this with iBus but I've not confirmed it yet. 1. Load attachment 613596 [details] 2. Type something in an editor with IME (don't commit it) 3. Click in the editor Actual result: Composition is canceled. Expected result: Composition is committed.
Assignee | ||
Comment 1•10 years ago
|
||
This is a bug of nsHTMLEditorEventListener::MouseDown().
Component: Event Handling → Editor
Comment 2•10 years ago
|
||
Good: https://hg.mozilla.org/integration/mozilla-inbound/rev/11fc11a90d6b Mozilla/5.0 (X11; Linux i686; rv:35.0) Gecko/20100101 Firefox/35.0 ID:20140925165440 Bad: https://hg.mozilla.org/integration/mozilla-inbound/rev/73b8079308bd Mozilla/5.0 (X11; Linux i686; rv:35.0) Gecko/20100101 Firefox/35.0 ID:20140925170540 Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=11fc11a90d6b&tochange=73b8079308bd
Assignee | ||
Comment 3•10 years ago
|
||
Thank you, Alice-san. I wonder how do you test it. When I try to test with mozregression on my Ubuntu 14.04 environment, IME doesn't work on Nightly :-( [Tracking Requested - why for this release]: This is a regression of 1065835 and this could break some web apps which use element.blur() and element.focus() hack for committing composition forcibly. The fix will be simple.
status-firefox34:
--- → unaffected
status-firefox35:
--- → affected
status-firefox36:
--- → affected
tracking-firefox35:
--- → ?
Comment 4•10 years ago
|
||
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #3) > Thank you, Alice-san. I wonder how do you test it. When I try to test with > mozregression on my Ubuntu 14.04 environment, IME doesn't work on Nightly :-( FYI, I am using tar.bz2 build on Ububtu12.04 32bit iBus+anthy, not mozregression tool.
Assignee | ||
Comment 5•10 years ago
|
||
Thank you for the information. It sounds you're hard worker! I'll file a bug of mozregression.
Assignee | ||
Comment 6•10 years ago
|
||
In nsEditorEventListener::MouseDown(), IMEStateManager::NotifyIME(REQUEST_TO_COMMIT) is called. However, if the editor is nsHTMLEditor, nsHTMLEditorEventListener::MouseDown() handles mousedown events first. At the first of the method, it checks if the mousedown event is fired on an element which is a descendant of editable root. However, in designMode, the editable root is <body>. Therefore, if mousedown event is fired on <html> element, the handler believes that the event is fired outside of it. Then, NotifyIME() isn't called. This is the cause of this bug. (Although, I guess there is another cause around nsGtkIMModule and TextComposition) In designMode editor, actual editable node is the document node. So, the result of the check is not expected result. Instead of IsDescendantOfEditorRoot(), nsHTMLEditorEventListener should use IsAcceptableInputEvent(). It checks any cases as expected for mouse events. At least for now, this is enough and I think this is the safest approach. # We need to uplift this patch to Aurora.
Attachment #8506129 -
Flags: review?(ehsan.akhgari)
Updated•10 years ago
|
Attachment #8506129 -
Flags: review?(ehsan.akhgari) → review+
Comment 7•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/030d8d468498
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Comment 8•10 years ago
|
||
Looks like this needs to be uplifted to Aurora to ensure we don't ship this regression, can you please nominate for uplift?
Assignee | ||
Comment 9•10 years ago
|
||
Yeah, I'll do that today. Thank you for the ping.
Flags: needinfo?(masayuki)
Keywords: regressionwindow-wanted
Assignee | ||
Comment 10•10 years ago
|
||
Comment on attachment 8506129 [details] [diff] [review] Patch Approval Request Comment [Feature/regressing bug #]: bug 1083067 [User impact if declined]: Typically, composing string with IME should be committed if user presses mouse button. However, this causes canceling the composition. I.e., if user expects to commit composition by mouse operation, this bug is a data loss bug. This may occur on rich text editor like web mail composer. [Describe test coverage new/current, TBPL]: I tested manually. For creating automated tests for this, we need to hack TextComposition with some risk which shouldn't be taken to uplift. [Risks and why]: The risk must be low. The old code checks if the clicked element is a descendant of editable root element. However, in designMode, comparing with the editable root element (<body> in this case) doesn't make sense because <html> element is also a part of editor. And IsAcceptableInputEvent() is used for filtering mouse button events in other mouse button event handlers of HTML editor. So, we can trust this alternative method. [String/UUID change made/needed]: Nothing.
Attachment #8506129 -
Flags: approval-mozilla-aurora?
Comment 11•10 years ago
|
||
Comment on attachment 8506129 [details] [diff] [review] Patch Thanks!
Attachment #8506129 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in
before you can comment on or make changes to this bug.
Description
•