348838 - can't delete a composed character with single Delete keypress

Status: NEW

(Core :: DOM: Editor, defect, P5)

Description

[Akira TAGOH]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; ja; rv:1.8.0.6) Gecko/20060808 Fedora/1.5.0.6-2.fc5 Firefox/1.5.0.6 pango-text
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; ja; rv:1.8.0.6) Gecko/20060808 Fedora/1.5.0.6-2.fc5 Firefox/1.5.0.6 pango-text

A composed character that is used in such as Indic Languages, can't be deleted by single Delete key press. they are sequentially composed with the base consonant and some consonant conjuncts. deleting consonant conjunct with Backspace key is what we expected. that's ok. they can continue to type another one after that then.
However deleting a base consonant only with Delete key isn't what we expected. there isn't much point in keeping the consonant conjuncts themselves in the language aspect.

Reproducible: Always

Steps to Reproduce:
1.open sample.html
2.move the cursor to the top of line.
3.press Delete key.

Actual Results:  
the consonant conjuncts are still there.

Expected Results:  
the whole composed character should be deleted.

Comment 1

[Akira TAGOH]

Attachment: testcase

Comment 2

[Akira TAGOH]

Attachment: proposed patch to fix this issue

attached a patch to solve this issue. can someone review this patch?

Comment 3

[Akira TAGOH]

That patch also fixes the same problem on thunderbird. and it also works fine on midas.

Comment 4

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 234185 [details] [diff] [review]
proposed patch to fix this issue

Daniel, can you give this patch a look?  Thanks.

Comment 5

[Simon Montagu :smontagu]

Uri: since delete is always a logical operation, I'm fairly sure that the test for (caretBidiLevel & 1) in the patch is wrong. Would it be enough just to remove that, or does there need to be a new CharacterExtendForDelete() method in nsISelectionController similar to WordExtendForDelete() from bug 344226?

Comment 6

[Uri Bernstein]

(In reply to comment #5)

Yes, a CharacterExtendForDelete() method would be necessary. (The test in the patch is wrong anyway, because the caret bidi level doesn't necessarily reflect the direction of the text it's on).

See aaronlev's original patches on bug 344423 (e.g. attachment 232268 [details] [diff] [review]) which added that method. I effectively shot down that idea then, because I was a bit scared of the implications, and because I believed that using the content (not the frames) was a better approach. I'm no longer sure about that.

Anyway, I think that using the content tree for backwards-deleting, but the frame tree for forward-deleting is a bad idea, which will likely cause unwanted inconsistencies (such as the treatment of collapsed whitespace and unrenderable control characters).

See also bug 332739.

Comment 7

[Wayne Mery (:wsmwk)]

Comment on attachment 234185 [details] [diff] [review]
proposed patch to fix this issue

canceling review based on comment 6

Comment 8

[Makoto Kato [:m_kato]]

Confirmed on 52.

Comment 9

[Jens Stutte [:jstutte]]

Bulk-downgrade of unassigned, untouched DOM/Storage bug's priority.

If you have reason to believe, this is wrong, please write a comment and ni :jstutte.