582181 - Incorrect rendering of directionality in mixed LTR and RTL texts

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Huji]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2) Gecko/20100115 Firefox/3.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2) Gecko/20100115 Firefox/3.6

If you have a textarea which is in RTL layout, and you put in a piece of text which is a mixture of RTL and LTR content, if the LTR content starts with specific characters (like braces, curly braces or < or >), the directionality of the character is affected by the text "before" it, not the text on the same line. This causes confusion.

Reproducible: Always

Steps to Reproduce:
1. Create a page in RTL layout nd put a textarea in it which also flows from right to left. As a shortcut, you can use Persian Wikipedia's Edit Box ( http://fa.wikipedia.org/w/index.php?title=testpage&action=edit&section=new )
2. Inside the text area, paste this sample text:

Blah blah
فلان فلان
<ref>ooo</ref>
<references />


Actual Results:  
The opening < for the REF tag and the REFERENCES tag is not shown in the desired place.


See this screenshot please: http://upload.wikimedia.org/wikipedia/test/e/ed/Rtlproblem2.png

The top two windows belong to IE and Google Chrome (latest versions), and the bottom box belongs to Firefox. You can clearly get the difference in the rendering of the text.

This bug has been present in Firefox at least since version 2.5 (IIRC), and apparently no one has bothered to report it.

Comment 1

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Is this bug and bug 505685 duplicate?

Comment 2

[ebrahim]

i know this was related to Bug 263359 that today fixed :)

Comment 3

[Tyler Downer [He/Him]]

Reporter, are you still seeing this issue with Firefox 4.0.1 or later in safe mode or a fresh profile? If not, please close. These links can help you in your testing.
http://support.mozilla.com/kb/Safe+Mode
http://support.mozilla.com/kb/Managing+profiles

Comment 4

[ebrahim]

Sadly, bug 263359 is backed out so this can be seen again, (in versions this was committed you couldn't find that, same "Gecko/20110413 Firefox/6.0a1")

Comment 5

[ebrahim]

Attachment: Correct rendering

about above comment

Comment 6

[Huji]

(In reply to comment #3)
> Reporter, are you still seeing this issue with Firefox 4.0.1 or later in
> safe mode or a fresh profile? If not, please close. These links can help you
> in your testing.
> http://support.mozilla.com/kb/Safe+Mode
> http://support.mozilla.com/kb/Managing+profiles

Yes, the bug is still present in Firefox 4.0.1 in safe mode and using a fresh profile.

Comment 7

[Tyler Downer [He/Him]]

This bug was reported using a pre-release version of Firefox 4. Now that Firefox 4.0.1 final has been released, can you please update and retest your bug? A fresh profile would be a good starting place to test, 
http://support.mozilla.com/kb/Managing+profiles. If you continue to see the issue, can you please update this bug with your results?

Filter: firefox4prebugsunco

Comment 8

[Huji]

Tested with a clean profile on Firefox 4.0.1 in Ubuntu and in Win, and the bug still persists.

Comment 9

[(no longer active)]

Can you please try it in the latest nightly now that bug 263359 is fixed?

Comment 10

[ebrahim]

Ehsan, I downloaded last nightly, it is generally fixed but in this testcase:
data:text/html;charset=utf-8,<textarea style="direction:rtl">فارسی%0A[[en:Farsi]]%0A</textarea>
please do a backspace in last line of textarea (remove last enter of textarea), something going wrong I thing. it is too much better from previous Firefox, but i think this little problem can be fixed also

Comment 11

[Huji]

I also see another problem. Using the original example:

Blah blah
فلان فلان
<ref>ooo</ref>
<references />

If you put the cursor at the end of the last line, hit BACKSPACE once (so the > will be removed) and then type ">" again, it will not be shown correctly.

Comment 12

[(no longer active)]

I can reproduce both comment 10 and 11.  Simon, I think that dynamic changes somehow screw up the bidi calculations here...

Comment 13

[Simon Montagu :smontagu]

Attachment: Patch

Comment 14

[Simon Montagu :smontagu]

Attachment: Mochitests

Comment 15

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

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

Review of attachment 539146 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/nsBidiPresUtils.cpp
@@ +650,5 @@
>      || !aFrame->IsFrameOfType(nsIFrame::eBidiInlineContainer);
>  }
>  
>  void
> +nsBidiPresUtils::AppendFrameAndAdvance(nsIFrame*& aFrame,

Shouldn't this be called AdvanceAndAppendFrame?

I think aFrame and aNextSibling should be nsIFrame** so that it's clear in the callers that the frame references are modified.

Comment 16

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

Comment on attachment 539147 [details] [diff] [review]
Mochitests

Review of attachment 539147 [details] [diff] [review]:
-----------------------------------------------------------------

Comment 17

[Simon Montagu :smontagu]

http://hg.mozilla.org/mozilla-central/rev/ec3718e17035
http://hg.mozilla.org/mozilla-central/rev/dcc03e00b32a

Comment 18

[Simon Montagu :smontagu]

Attachment: Patch with comments addressed

Oops, I thought you had marked this as r+ with comments, and I checked it in already. Can you r+ it retroactively (or not, and I'll back out)?

Comment 19

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

Comment on attachment 539803 [details] [diff] [review]
Patch with comments addressed

Review of attachment 539803 [details] [diff] [review]:
-----------------------------------------------------------------