Closed Bug 317782 Opened 19 years ago Closed 19 years ago

A character is deleted on HTML Editor when running IME reconvertion with non-selected text


(Core :: DOM: Editor, defect)

Not set





(Reporter: masayuki, Assigned: masayuki)



(5 keywords, Whiteboard: [tjp-dl])


(1 file)

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.
Attached patch Patch rv1.0Splinter Review
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.
Attachment #204171 - Flags: review?(timeless)
Attachment #204171 - Flags: review?(timeless) → review+
Attachment #204171 - Flags: superreview?(bzbarsky)
I'm sorry, but this needs review or at least module owner approval from someone resembling an editor peer before I can sr it.

What? This patch was already reviewed by timeless. He is editor peer.
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 on attachment 204171 [details] [diff] [review]
Patch rv1.0


Thanks, I'm changing to Simon Fraser.
Attachment #204171 - Flags: superreview?(bzbarsky) → superreview?(sfraser_bugs)

This patch fixes bug 296686 too. That bug is very serious usability issue of Japanese EGBRIDGE users. Please sr it.
Flags: blocking1.8.1?
Attachment #204171 - Flags: superreview?(sfraser_bugs) → superreview+
checked-in to Trunk, Thanks!
Closed: 19 years ago
Resolution: --- → FIXED
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
Attachment #204171 - Flags: approval1.8.1?
Attachment #204171 - Flags: approval1.8.0.1?
Comment on attachment 204171 [details] [diff] [review]
Patch rv1.0

Too late for, but thanks for flagging it so we can consider it for .0.2
Attachment #204171 - Flags: approval1.8.0.2?
Attachment #204171 - Flags: approval1.8.0.1?
Attachment #204171 - Flags: approval1.8.0.1-
Attachment #204171 - Flags: branch-1.8.1?(timeless)
Attachment #204171 - Flags: branch-1.8.1?(timeless) → branch-1.8.1+
Attachment #204171 - Flags: approval1.8.1?
Flags: blocking1.8.1? → blocking1.8.0.2?
Keywords: fixed1.8.1
Target Milestone: mozilla1.9alpha → mozilla1.8.1
Flags: blocking1.8.0.2? → blocking1.8.0.2+
Comment on attachment 204171 [details] [diff] [review]
Patch rv1.0

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #204171 - Flags: approval1.8.0.2? → approval1.8.0.2+
-> v.
Whiteboard: [tjp-dl]
You need to log in before you can comment on or make changes to this bug.