Closed Bug 205235 Opened 17 years ago Closed 16 years ago

RTL first line overlaps "float: left" elements

Categories

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

x86
All
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: neokuwait, Assigned: ilya.konstantinov+future)

References

()

Details

(Keywords: fixed-aviary1.0, fixed1.7, rtl)

Attachments

(4 files)

Gecko/20030429

TO REPRODUCE:

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

OR:

View testcase
Attached file simplified testcase
I see this on LInux 2003050809 and on my gtk2/xft CVS build
Confirming for now, but I think I've seen this before, so it may be a dupe.
Status: UNCONFIRMED → NEW
Ever confirmed: true
*** Bug 230231 has been marked as a duplicate of this bug. ***
(In reply to comment #2)
> I see this on LInux 2003050809 and on my gtk2/xft CVS build

then this is OS: All, no?
OS: Windows XP → All
Arabic or Hebrew text is not actually required to reproduce this bug; only an
RTL paragraph.
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!
BTW, bug appears also with align="left" attribute for the image (instead of
style="float:left").
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. 
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.
Assignee: mkaply → mozilla-bugzilla
Status: NEW → ASSIGNED
Attachment #151179 - Flags: superreview?(dbaron)
Attachment #151179 - Flags: review?(roc)
Attachment #151179 - Flags: superreview+
Attachment #151179 - Flags: review+
Attachment #151179 - Flags: superreview?(dbaron)
Attachment #151179 - Flags: review?(roc)
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
is this fixed on the 1.7 branch, or just the trunk?
(In reply to comment #11)
> is this fixed on the 1.7 branch, or just the trunk?

trunk only
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.
I second the request. This is after all a trivial patch with no harmful
side-effects.
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 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 on attachment 151179 [details] [diff] [review]
Change nsLineLayout::UpdateFrames to work for RTL spans

count me in :-)

Asking approval-aviary
Attachment #151179 - Flags: approval-aviary?
Attachment #151179 - Flags: approval1.7.3?
Comment on attachment 151179 [details] [diff] [review]
Change nsLineLayout::UpdateFrames to work for RTL spans

a=mkaply for aviary and 1.7.3
Attachment #151179 - Flags: approval1.7.3?
Attachment #151179 - Flags: approval1.7.3+
Attachment #151179 - Flags: approval-aviary?
Attachment #151179 - Flags: approval-aviary+
Keywords: fixed1.7
*** Bug 254962 has been marked as a duplicate of this bug. ***
verified, fixed on branch since the 2004-08-10.
*** Bug 255049 has been marked as a duplicate of this bug. ***
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.
Mass-assigning the new rtl keyword to RTL-related (see bug 349193).
Keywords: rtl
Component: Layout: BiDi Hebrew & Arabic → Layout: Text
QA Contact: zach → layout.fonts-and-text
You need to log in before you can comment on or make changes to this bug.