Closed Bug 1109465 Opened 5 years ago Closed 5 years ago
Committing text with IME at an end of line causes odd undo transactions
1. Load the URL. 2. Type Foo for first line text. 3. Type Enter for line break. 4. Type Bar for second line text. 5. Move caret to the end of first line. 6. Use IME and commit a character. 7. Do Undo. Actual result: The value becomes "Fooar" Expected result: Should be "Foo\nBar" Aurora doesn't have this bug...
[Tracking Requested - why for this release]: Due to new regression in 37 and dataloss of undo/redo transaction.
hum, I can reproduce the problem on Aurora36.0a2 as well as Firefox34, but not on Firefox33. Regression window(m-i) Good: https://hg.mozilla.org/integration/mozilla-inbound/rev/e0461ce2e137 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0 ID:20140829030900 Bad: https://hg.mozilla.org/integration/mozilla-inbound/rev/8e47222be374 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0 ID:20140829044401 Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e0461ce2e137&tochange=8e47222be374 Regressed by: Bug 1056166
In local build Last Good: e787a0f1c6f2 First Bad: 8e47222be374 Triggered by 8e47222be374 Aryeh Gregor — Bug 1056166 part 6 - Clean up IMETextTxn; r=ehsan
I am not available to do any work until March/April, so if no one else wants to look at this, it might be a good idea to try backing out the patch.
Regression, tracking. Ehsan, can you help on this?
Masayuki, do you have cycles by any chance?
Flags: needinfo?(ehsan.akhgari) → needinfo?(masayuki)
I'm very busy for designing new IME API for B2G. However, I'd like to look more next week if I could... If somebody can work on this, feel free to take this.
Ehsan - we're going to build our last desktop beta today but we could take a backout for RC next week if it's low risk. Can you find time to help with this? The alternative is to wontfix for 35 since we did already ship but since we know the solution, it would be great to get a fix before release for this recent regression.
Bug 1056166 broken the interface maps for IMETextTxn and InsertTextTxn. I have a fix.
Assignee: nobody → ehsan
The original change in bug 1056166 used NS_INTERFACE_MAP_ENTRY_AMBIGUOUS with the wrong order for argument names. But more importantly, we cannot use NS_INTERFACE_MAP_ENTRY_AMBIGUOUS here because the cast from the concrete class to nsISupports* will remain ambiguous even when casting first through nsITransaction, so we need to write the implementation without using any macros.
Comment on attachment 8542573 [details] [diff] [review] Fix the interface maps of IMETextTxn and InsertTextTxn Approval Request Comment [Feature/regressing bug #]: Bug 1056166. [User impact if declined]: See comment 0. [Describe test coverage new/current, TBPL]: Tested locally, and also wrote an automated test. [Risks and why]: Extremely low risk. This basically broke QueryInterface() for IMETextTxn and InsertTextTxn, and therefore this code stopped working: <http://dxr.mozilla.org/mozilla-central/source/editor/libeditor/PlaceholderTxn.cpp#125>. Failing to merge transactions is expected to break undo in the way described in comment 0. [String/UUID change made/needed]: None.
Comment on attachment 8542573 [details] [diff] [review] Fix the interface maps of IMETextTxn and InsertTextTxn No need to put on mozilla-release, we won't chemspill 34 at this stage.
Attachment #8542573 - Flags: approval-mozilla-release?
Attachment #8542573 - Flags: approval-mozilla-release-
Attachment #8542573 - Flags: approval-mozilla-beta?
Attachment #8542573 - Flags: approval-mozilla-beta+
Attachment #8542573 - Flags: approval-mozilla-aurora?
Attachment #8542573 - Flags: approval-mozilla-aurora+
The test fails on beta for some reason <https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&revision=936d2a5ac91b> so I disabled it: https://hg.mozilla.org/releases/mozilla-beta/rev/a6c24971edc2
(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #16) > The test fails on beta for some reason > <https://treeherder.mozilla.org/#/jobs?repo=mozilla- > beta&revision=936d2a5ac91b> so I disabled it: > > https://hg.mozilla.org/releases/mozilla-beta/rev/a6c24971edc2 FWIW I verified the fix on beta manually.
You need to log in before you can comment on or make changes to this bug.