514156 - adding non-spacing characters to an empty input boxes and adding characters right after may hide the text box content

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Tomer Cohen :tomer]

Attachment: testcase

Steps to reproduce:
A. Type a non-spacing character into an input box. For example, add the character U+05B6 Hebrew Point Segol ("ֶ"). 
B. Type any character. It can be Hebrew character and even a Latin letter.
C. Type another regular character. 

Actual result:
1. When adding a non-spacing character, you should see the character appearing in the desired position.
2. After adding one regular character you should see it joining to the previous character. 
3. After adding another regular character, the content would disappear, which is not what should happen.
4. When adding another character (total of four) and the content would re-appear. More characters won't break the display.




Rumors say that smontagu already have a patch for this issue. :)

Comment 1

[Simon Montagu :smontagu]

Attachment: Patch

When testing whether the frame associated with a content node has width, we only check the first frame. For the case here where bidi resolution splits the frame after the first character, which happens to be a non-spacing character, we have to check the continuation frames as well.

Comment 2

[Simon Montagu :smontagu]

Attachment: Patch v.2

Comment 3

[Simon Montagu :smontagu]

Comment on attachment 398332 [details] [diff] [review]
Patch

The tests in this version of the patch are still valid, but the change to nsEditor.cpp is obsoleted by attachment 406004 [details] [diff] [review]

Comment 4

[Peter Van der Beken [:peterv]]

Comment on attachment 406004 [details] [diff] [review]
Patch v.2

>+    while (resultFrame) {
>+      if (resultFrame->GetStateBits() & NS_FRAME_IS_DIRTY) // we can only trust width data for undirty frames
>+      {
>+        // In the past a comment said:
>+        //   "assume all text nodes with dirty frames are editable"
>+        // Nowadays we use a virtual function, that assumes TRUE
>+        // in the simple editor world,
>+        // and uses enhanced logic to find out in the HTML world.
>+        return IsTextInDirtyFrameVisible(aNode);

I was wondering if it's ok to always return early here, instead of returning only if IsTextInDirtyFrameVisible is true. I think I've convinced myself it's ok because nsHTMLEditor::IsTextInDirtyFrameVisible returning false means that there's only whitespace.

Comment 5

[Simon Montagu :smontagu]

http://hg.mozilla.org/mozilla-central/rev/5481246948c6

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Is this a regression from bug 504524? If so, why would it block 1.9.2, since 504524 was not fixed there?

Comment 7

[Simon Montagu :smontagu]

(In reply to comment #6)
> Is this a regression from bug 504524?

No. The regression range is 2007-05-06 - 20007-05-07. I don't see anything in http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-05-05+20%3A00&maxdate=2007-05-07+08%3A00&cvsroot=%2Fcvsroot that would have caused this, but maybe bug 375716 or bug 378784 exposed it somehow