205235 - RTL first line overlaps "float: left" elements

Status: RESOLVED FIXED

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

Description

[Thamer Mahmoud]

Gecko/20030429

TO REPRODUCE:

1. go to URL
2. scroll down until you see a blue quote box

OR:

View testcase

Comment 1

[Thamer Mahmoud]

Attachment: simplified testcase

Comment 2

[Jesiah S]

I see this on LInux 2003050809 and on my gtk2/xft CVS build

Comment 3

[Simon Montagu :smontagu]

Confirming for now, but I think I've seen this before, so it may be a dupe.

Comment 4

[Alexey Chernyak]

*** Bug 230231 has been marked as a duplicate of this bug. ***

Comment 5

[Tsahi Asher]

(In reply to comment #2)
> I see this on LInux 2003050809 and on my gtk2/xft CVS build

then this is OS: All, no?

Comment 6

[Ilya Konstantinov]

Attachment: More simple testcase (English-only)

Arabic or Hebrew text is not actually required to reproduce this bug; only an
RTL paragraph.

Comment 7

[Ilya Konstantinov]

Attachment: Another interesting testcase

This one is stripped from BODY and HTML (non-standard, but that's not the point
here), down to the bare code which still reproduces the problem.

There are two tests in the testcase. The second one doesn't have an empty #text
element before the float, and *doesn't* exhibit the bug!

Comment 8

[Eyal Rozenberg]

BTW, bug appears also with align="left" attribute for the image (instead of
style="float:left").

Comment 9

[Ilya Konstantinov]

Re Eyal Roz: align="left" is the HTML 3.2 way of saying float:left. It's the 
same. 
 
-- 
And now, to the interesting part: 
 
The layout we have here is: 
<RTL block> 
[empty text frame] 
[float frame] 
[full text frame] 
</RTL block> 
 
The nsLineLayout will pass over those 3 frames: 
This first frame's boundary (mBounds) begins at 0x0 (most-left part of the 
line; RTL is irrelevant here, since we're talking about a visual boundary). 
The second frame is a float, and is rendered out-of-line. While placing it, the 
nsLineLayout's general boundary is shrinked. Then nsLineLayout::UpdateFrames is 
called. Since the previous text frame was empty, Mozilla considers it Ok to 
keep the float on this line. 
The third frame's boundary begins at 200x0 (200px to the left, right after our 
200px floatie). 
 
Thing is, UpdateFrames does nothing when the block is RTL. It has a TODO for 
handling RTL. So, in our RTL block, it doesn't updates the empty text frame's 
boundary. From what I see, since UpdateFrames only updates a visual boundary 
(thus, not direction-specific), it should use the same code for RTL too. 
 
However, one thing still puzzles me. The third frame (the full text frame) *is* 
bounded from 200x0, so apparently the empty text frame should be placed at 0x0 
but the full text frame should begin at 200x0 (and then everything would look 
pretty) -- whereas, in reality, the full text frame is placed right after the 
empty text frame. Is there something in layouting which joins the boundary of 
adjacent text frames? 
 
-- 
 
To sum it up, I've made UpdateFrames use the same code for both LTR and RTL 
blocks, and it behaves allright now. 

Comment 10

[Ilya Konstantinov]

Attachment: Change nsLineLayout::UpdateFrames to work for RTL spans

No need for a separate branch for RTL spans. Bounds are visual and left-top
oriented, so adding a left float requires updating them, no matter if we're in
RTL or LTR.

Comment 11

[Tsahi Asher]

is this fixed on the 1.7 branch, or just the trunk?

Comment 12

[Asaf Romano (gone)]

(In reply to comment #11)
> is this fixed on the 1.7 branch, or just the trunk?

trunk only

Comment 13

[Uri Bernstein]

Any chance for the fix to be checked in to the Aviary branch?
This bug affects many Hebrew pages (including articles on the Haaretz site
(www.haaretz.co.il)), and it would be easier for me to convince people to switch
to Firefox if this goes away.

Comment 14

[Eyal Rozenberg]

I second the request. This is after all a trivial patch with no harmful
side-effects.

Comment 15

[Reuven Gonen]

I add my self to the request. this bug is very annoying, especially in news
sites. one of the sites is www.nfc.co.il and appears in other web sites as well.
i got already a few complains from hebrew users about this issue. we have enough
broblems with sites being compatible with mozilla without adding our own bugs.

Comment 16

[Jungshik Shin]

Comment on attachment 151179 [details] [diff] [review]
Change nsLineLayout::UpdateFrames to work for RTL spans

Ilya,

It's been baked in the trunk  for a month and it seems that no regression has
been reported. Why don't you ask for approval-aviary and approval-1.7?

Comment 17

[Asaf Romano (gone)]

Comment on attachment 151179 [details] [diff] [review]
Change nsLineLayout::UpdateFrames to work for RTL spans

count me in :-)

Asking approval-aviary

Comment 18

[Mike Kaply [:mkaply]]

Comment on attachment 151179 [details] [diff] [review]
Change nsLineLayout::UpdateFrames to work for RTL spans

a=mkaply for aviary and 1.7.3

Comment 19

[Scott MacVicar]

*** Bug 254962 has been marked as a duplicate of this bug. ***

Comment 20

[Reuven Gonen]

verified, fixed on branch since the 2004-08-10.

Comment 21

[Simon Montagu :smontagu]

*** Bug 255049 has been marked as a duplicate of this bug. ***

Comment 22

[Tsahi Asher]

we have a problem: now, lists that are opposed to a left floating object (i
tested with a table and an image) are extremely outdented. see bug 262195.

Comment 23

[(no longer active)]

Mass-assigning the new rtl keyword to RTL-related (see bug 349193).