Overflowing content in fixed-width RTL blocks overflows to the right instead of to the left

RESOLVED FIXED in mozilla1.9alpha1

Status

()

RESOLVED FIXED
14 years ago
11 years ago

People

(Reporter: uriber, Assigned: uriber)

Tracking

({rtl, testcase})

Trunk
mozilla1.9alpha1
rtl, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(4 attachments, 1 obsolete attachment)

(Assignee)

Description

14 years ago
When an RTL block with a specified width contains a line of text or an image, which is wider than the width of the block, the text or image overflows to the right of the containing block, instead of to the left.

If the overflowing content is itself a block with a specified width, the problem only occurs if the inner block has "margin:auto;" (see bug 96394).

Bug 202386 also seems like a special case of this bug (where the containing block is the body element, and the overflowing content is a table).

The example in the URL field is a very complex testcase. If you reduce the window width so that the image does not fit, it will overflow to the right and cover the menus.
I'll attach a much simpler testcase soon.
(Assignee)

Comment 1

14 years ago
Posted file testcase
The first two blocks demonstrate the problem with overflowing text (they behave the same, regardless of the directionality of the text itslef).
The third block demonstrates the same problem with an image.
The fourth block has a fixed-width DIV as content, and does not exhibit the problem.
The fifth block is the same as the fourth, but with "margin:auto;", causing the problem to re-appear.
(Assignee)

Comment 2

14 years ago
This is the same testcase, but with overflow:auto on the container divs. The problem basically remains, but notice:
- The scroll thumbs are initially positioned incorrectly (the divs themselves appear scrolled all the way to the right, but the scrollbars indicate that they are somewhat to the left)
- The fourth block, which seemed to be OK in the previous testcase, lacks a scrollbar altogether (this might be bug 6976).

Comment 3

14 years ago
This is probably not a bidi problem but more that we can't scroll left if content overflows there.
If we fix this then in containers with scrollbars we won't be able to scroll to the left-overflowing content.
... unless of course we make it possible to scroll to the left of the origin, which should indeed be possible, and probably wouldn't be that hard as it would only touch nsGfxScrollFrame.cpp and nsScrollPortView.cpp, I think. Just make scroll offset (0,0) correspond to the top-left of the scrolled frame's overflow area instead of the scrolled frame's origin.
But you'd need to make sure that the default scroll position had the child scrolled so the child's origin is at the scrolled frame's origin.

Comment 7

14 years ago
I'm having trouble figuring out the exact relationships between this bug, bug 6976 and bug 192767. Are they all separate issues or are some of them dupes of others? What blocks what? etc. 

In comment 0 I'm not reading anything about scrolling yet the discussion seems to revolve around that.
(Assignee)

Comment 8

14 years ago
(In reply to comment #7)
> I'm having trouble figuring out the exact relationships between this bug, bug
> 6976 and bug 192767. Are they all separate issues or are some of them dupes of
> others? What blocks what? etc.  

As far as I understand, this is a separate bug, which is not directly related to scrolling (hence no scrollbars mentioned in comment #0 or in the first testcase). However, as ROC pointed out in comment #4, fixing this bug will result in additional scrolling problems, which, I think, are really the same as bug 6976 / bug 192767 (which I think are actually dupes of each other).

ROC - is this accurate? If so, should this bug be marked as depending on bug 6976?
(Assignee)

Updated

14 years ago
Depends on: 6976

Updated

14 years ago
Keywords: testcase
Summary: Oveflowing content in fixed-width RTL blocks overflows to the right instead of to the left → Overflowing content in fixed-width RTL blocks overflows to the right instead of to the left
one question
please resize the width of
http://egytopia.wikicities.com/index.php?title=%D8%B9%D9%84%D9%89_%D8%AE%D9%84%D9%81%D9%8A%D8%A9_%D8%A3%D8%B2%D9%85%D8%A9_%D9%85%D8%AD%D8%B1%D9%85_%D8%A8%D9%83&oldid=74
The images will overlap with the "adds" area.

Is this the same bug? If not which one?

Thanks for your feedback in advance.
best regards reinhardt [[user:gangleri]]
(Assignee)

Comment 11

13 years ago
Posted patch patchSplinter Review
This patch fixes the top three divs in the attached testcase (the fifth one is probably a separate issue - bug 96394).

As roc noted in comment #4, scrollbars are indeed missing in the overflow:hidden case (and in the textarea case). However, applying dbaron's patch for bug 192767 fixes this problem.

The line I removed from nsLineLayout::PlaceFrame() was added there as part of the fix for bug 169620. I don't really understand why it had to be added there (frankly, I don't really understand a lot of this code), but removing it does not regress that bug. I had to remove it in order to be able to detect and handle the overflow condition in nsLineLayout::HorizontalAlignFrames().

I removed "// XXXldb What if it's less than 0??" because I'm now handling that case (although, apparently it only needs handeling in the RTL case). Note that before I removed the MAX(...) line from nsLineLayout::PlaceFrame(), this could not happen: remainingWidth was never negative.
Attachment #213593 - Flags: review?(dbaron)
(Assignee)

Updated

13 years ago
No longer blocks: 96394
(Assignee)

Updated

13 years ago
No longer blocks: 202386
The patches to bug 192767 (scrolling to right), bug 96394 (block overflow should go to right), and bug 318116 (inline overflow should go to right) should all be landed at about the same time.

Comment 13

13 years ago
Posted patch patch with padding (obsolete) — Splinter Review
I'm sorry, uri.  I posted this patch since it seems that a problem is shown in the display of left-padding. If your patch is corrected, I think that it is good.

Comment 14

13 years ago
Does mComputedBorderPadding already account for GetSkipSides, or do you need to check that?
(Assignee)

Comment 16

13 years ago
(In reply to comment #13)
> Created an attachment (id=214984) [edit]
> patch with padding
> 
> I'm sorry, uri.  I posted this patch since it seems that a problem is shown in
> the display of left-padding. If your patch is corrected, I think that it is
> good.
> 

Not that I mind, but as far as I can tell, the testcase you posted looks exactly the same with your patch as with mine (and seems correct in both cases).

Comment 17

13 years ago
(In reply to comment #16)
> Not that I mind, but as far as I can tell, the testcase you posted looks
> exactly the same with your patch as with mine (and seems correct in both
> cases).

Really? I am sorry, since I tested using source code which is not latest.

Updated

13 years ago
Attachment #214984 - Attachment is obsolete: true
Attachment #213593 - Flags: superreview?(roc) → superreview+
(Assignee)

Updated

13 years ago
Assignee: mozilla → uriber
Target Milestone: --- → mozilla1.9alpha
(Assignee)

Comment 18

13 years ago
Checked in:
Checking in layout/generic/nsLineLayout.cpp;
/cvsroot/mozilla/layout/generic/nsLineLayout.cpp,v  <--  nsLineLayout.cpp
new revision: 3.231; previous revision: 3.230
done
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
(Assignee)

Updated

13 years ago
Blocks: 247691

Updated

13 years ago
Blocks: 331562

Comment 19

11 years ago
Mass-assigning the new rtl keyword to RTL-related (see bug 349193).
Keywords: rtl

Updated

11 years ago
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.