Closed Bug 1266165 Opened 4 years ago Closed 3 years ago

Caret disappears on redo when using IME

Categories

(Core :: Editor, defect)

45 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: aaron.nance, Assigned: oshinjyo14)

Details

(Keywords: inputmethod)

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/49.0.2623.112 Safari/537.36

Steps to reproduce:

On windows 7 using FF 45.0.2. Steps to reproduce: 

Here is the HTML I am using: 

<html>
<head></head>
<body>
<textarea style="width: 300px; height: 150px;"></textarea>
</body>
</html>


Using the IME Keypad enter several lines of Pinyin text into the text area. 
Press Ctrl-Z to undo several lines. 
Press Ctrl-Y to redo. 


Actual results:

On redo, the caret disappears and does not reappear. 


Expected results:

The caret should not disappear.
Summary: Cursor disappears on unod when using IME → Cursor disappears on redo when using IME
OS: Unspecified → Windows 7
Hardware: Unspecified → x86
Component: Untriaged → Internationalization
Product: Firefox → Core
Component: Internationalization → Editor
Keywords: inputmethod
Reproduced on Mac too.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 7 → All
Hardware: x86 → All
Summary: Cursor disappears on redo when using IME → Caret disappears on redo when using IME
This must be easy to fix. When redoing composition, CompositionTransaction::DoTransaction() must be called. However, it tries to restore selection from mRanges.  However, it must be nullptr if it was merged with transaction for committing composition.
https://dxr.mozilla.org/mozilla-central/rev/24763f58772d45279a935790f732d80851924b46/editor/libeditor/CompositionTransaction.cpp#57,73

Then, it must hit here:
https://dxr.mozilla.org/mozilla-central/rev/24763f58772d45279a935790f732d80851924b46/editor/libeditor/CompositionTransaction.cpp#280-282,285,291

So, we should add a special case when mRanges is either nullptr or empty.
(This may be useful at Gecko Hands-on in Mozilla Japan Office.)
Assignee: nobody → oshinjyo14
Status: NEW → ASSIGNED
Attachment #8785532 - Flags: review?(masayuki)
Comment on attachment 8785532 [details] [diff] [review]
CompositionTransaction::SetIMESelection() should not hide caret when redo

>+    // However, when there is no range, we should keep showing caret.
>+    if (!countOfRanges) {

This is wrong, should be |if (countOfRanges)|. I'll modify it and land it.

Thank you for your contribution!
Attachment #8785532 - Flags: review?(masayuki) → review+
https://hg.mozilla.org/mozilla-central/rev/3dbe68e9fe81
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.