Closed Bug 318116 Opened 15 years ago Closed 15 years ago

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

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9alpha1

People

(Reporter: uriber, Assigned: uriber)

References

()

Details

(Keywords: rtl, testcase)

Attachments

(4 files, 1 obsolete file)

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.
Attached 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.
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).
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.
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.
(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?
Depends on: 6976
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]]
Attached 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)
No longer blocks: 96394
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.
Attachment #213593 - Flags: superreview?(roc)
Attachment #213593 - Flags: review?(dbaron)
Attachment #213593 - Flags: review+
Attached 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.
Attached file testcase for padding
Does mComputedBorderPadding already account for GetSkipSides, or do you need to check that?
(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).
(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.
Attachment #214984 - Attachment is obsolete: true
Attachment #213593 - Flags: superreview?(roc) → superreview+
Assignee: mozilla → uriber
Target Milestone: --- → mozilla1.9alpha
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
Closed: 15 years ago
Resolution: --- → FIXED
Blocks: 247691
Depends on: 331385
Blocks: 331562
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.