Last Comment Bug 317782 - A character is deleted on HTML Editor when running IME reconvertion with non-selected text
: A character is deleted on HTML Editor when running IME reconvertion with non-...
Status: VERIFIED FIXED
[tjp-dl]
: inputmethod, intl, jp-critical, verified1.8.0.2, verified1.8.1
Product: Core
Classification: Components
Component: Editor (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla1.8.1
Assigned To: Masayuki Nakano [:masayuki] (Mozilla Japan)
:
Mentors:
Depends on:
Blocks: 296686
  Show dependency treegraph
 
Reported: 2005-11-25 09:31 PST by Masayuki Nakano [:masayuki] (Mozilla Japan)
Modified: 2010-06-18 19:00 PDT (History)
3 users (show)
dveditz: blocking1.8.0.2+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch rv1.0 (8.31 KB, patch)
2005-11-25 11:03 PST, Masayuki Nakano [:masayuki] (Mozilla Japan)
timeless: review+
sfraser_bugs: superreview+
timeless: approval‑branch‑1.8.1+
dveditz: approval1.8.0.1-
dveditz: approval1.8.0.2+
Details | Diff | Review

Description Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-11-25 09:31:58 PST
On HTML editor, if we try to reconvert by IME when the text is not selected, the next character from the caret is deleted. It seems we pressed delete key.
This is not reproduced on Text editor.
Comment 1 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-11-25 11:03:50 PST
Created attachment 204171 [details] [diff] [review]
Patch rv1.0

We have a bug in nsHTMLEditRules::WillDeleteSelection.
If aAction is nsIEditor::eNone, the function is running as with nsIEditor::ePrevious. Because the process is checking aAction is nsIEditor::eNext or not. So, we should return in early time if aAction is nsIEditor::eNone.

And we don't need to run nsPlaintextEditor::DeleteSelection if the selection text is empty.

And we don't need IME code in nsHTMLEditor. Becuase nsHTMLEditor::SetCompositionString and nsHTMLEditor::GetReconversionString are same as nsPlaintextEditor's methods.(nsHTMLEditor's methods are overriding nsPlaintextEditor's one.) These code are duplicated by bug 66290. We should have removed these methods in this time.
Comment 2 Boris Zbarsky [:bz] (Out June 25-July 6) 2005-11-25 23:18:44 PST
I'm sorry, but this needs review or at least module owner approval from someone resembling an editor peer before I can sr it.
Comment 3 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-11-25 23:26:51 PST
boris:

What? This patch was already reviewed by timeless. He is editor peer.
Comment 4 Boris Zbarsky [:bz] (Out June 25-July 6) 2005-11-25 23:35:04 PST
Doh!  I missed his name in the list...

I'll try to get to this before as soon as I can, but I really don't know anything about this code (which means I'll have to learn it) and I've already got a huge backlog of reviews to do before this one, so I don't know that I'll be able to do it soon enough (that is, before I lose net access; if I don't get to it before then, then it won't happen till 2006).  No matter what, I won't be able to sr this for at least a week, which means things will be _really_ tight as far as getting it done in time...

If at all possible, I would recommend seeking another sr, preferably one familiar with IME or editor code (Simon Fraser, perhaps?)
Comment 5 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-11-25 23:47:36 PST
Comment on attachment 204171 [details] [diff] [review]
Patch rv1.0

Boris:

Thanks, I'm changing to Simon Fraser.
Comment 6 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-11-25 23:52:14 PST
Simon:

This patch fixes bug 296686 too. That bug is very serious usability issue of Japanese EGBRIDGE users. Please sr it.
Comment 7 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-11-26 14:10:34 PST
checked-in to Trunk, Thanks!
Comment 8 Masayuki Nakano [:masayuki] (Mozilla Japan) 2006-01-11 08:17:02 PST
Comment on attachment 204171 [details] [diff] [review]
Patch rv1.0

This bug dosen't have any regression reports. So this patch has low risk.

But the fixed problem is serious for Mac EGBRIDGE(IME of Ja) users.
# When the users hit space key between inputted characters, the next character of caret is deleted by this bug.

Let's go to 1.8.1 and 1.8.0.1.
Comment 9 Daniel Veditz [:dveditz] 2006-01-17 11:10:38 PST
Comment on attachment 204171 [details] [diff] [review]
Patch rv1.0

Too late for 1.8.0.1, but thanks for flagging it so we can consider it for .0.2
Comment 10 Daniel Veditz [:dveditz] 2006-02-14 15:28:46 PST
Comment on attachment 204171 [details] [diff] [review]
Patch rv1.0

approved for 1.8.0 branch, a=dveditz for drivers
Comment 11 Masayuki Nakano [:masayuki] (Mozilla Japan) 2006-02-17 08:02:07 PST
-> v.

Note You need to log in before you can comment on or make changes to this bug.