Closed Bug 514156 Opened 15 years ago Closed 15 years ago

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


(Core :: DOM: Editor, defect)

Not set





(Reporter: tomer, Assigned: smontagu)



(Keywords: rtl)


(3 files)

Attached file 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. :)
Attached patch PatchSplinter Review
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.
Attachment #398332 - Flags: review?
Attachment #398332 - Flags: review? → review?(peterv)
Blocks: 504524
Attached patch Patch v.2Splinter Review
Assignee: nobody → smontagu
Attachment #398332 - Attachment is obsolete: true
Attachment #406004 - Flags: review?
Attachment #398332 - Flags: review?(peterv)
Attachment #406004 - Flags: review? → review?(peterv)
Comment on attachment 398332 [details] [diff] [review]

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]
Attachment #398332 - Attachment is obsolete: false
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.
Attachment #406004 - Flags: review?(peterv) → review+
Flags: blocking1.9.2?
Closed: 15 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Is this a regression from bug 504524? If so, why would it block 1.9.2, since 504524 was not fixed there?
(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 that would have caused this, but maybe bug 375716 or bug 378784 exposed it somehow
Flags: wanted1.9.2-
Flags: blocking1.9.2?
Flags: blocking1.9.2-
No longer depends on: 524284
You need to log in before you can comment on or make changes to this bug.