307158 - Vertical scrollbar covers RHS of content forcing horizontal scrollbar to apear

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[G Evans]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.10) Gecko/20050716 Firefox/1.0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b4) Gecko/20050901 Firefox/1.0+

Tested on Windows 2000, I cannot test this on Linux or Mac.

Works OK on the following browsers

  Firefox        1.06         Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:1.7.10) Gecko/20050716 Firefox/1.0.6

  Mozilla        1.7.11       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:1.7.11) Gecko/20050728
  Mozilla        1.7.5        Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:1.7.5) Gecko/20041217

  Opera          8.02         Build 7680
  Opera          8.00         Build 7561
  Opera          7.52         Build 3834
  Opera          7.23         Build 3227
  Opera          7.20         Build 3144

  Netscape       8.0.1        Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:1.7.5) Gecko/20050519 Netscape/8.0.1
  Netscape       7.1          Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:1.4) Gecko/20030624 Netscape/7.1
  Netscape       6.2          Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:0.9.4.1) Gecko/20020508 Netscape6/6.2.3

Fails on

  Firefox        Deer Park    Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:1.8b4) Gecko/20050901 Firefox/1.0+
  Firefox        Deer Park    Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:1.8b3) Gecko/20050712 Firefox/1.0+

  Firefox        Deer Park    Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:1.9a1) Gecko/20050902 Firefox/1.6a1
  Firefox        Deer Park    Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:1.9a1) Gecko/20050823 Firefox/1.6a1

  SeaMonkey      1.1a         Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:1.9a1) Gecko/20050902 SeaMonkey/1.1a

Layout not designed for IE or other browsers as yet. Alternative style sheets
will be designed for these.



A test page is at the following URL that illustrates the issue.

<a href="http://www.phaf.org/DeerPark/">http://www.phaf.org/DeerPark/</a>


I have experienced a problem with the Deer Park builds and SeaMonkey where a
horizontal scrollbar always appears. I have not experienced this in previous
builds neither does it occur in many other browsers. A list of the browsers I
have been able to test on is above.

The width of the box appears to be calculated without regard for the presence of
the vertical scroll bar and this means that the scroll bar overlies contents of
the box. A horizontal scroll bar is then generated that can be scrolled to make
this visible.

Previous releases calculate the width to exclude the vertical scroll bar and do
not generate the horizontal scroll bar, as I would expect to happen. The idea
being to have the text flow into the box then be scrolled down. Padding would
not help as this would leave the unnecessary horizontal scroll bar and would
cause problems in other browsers.

This will cause compatibility issuses between different versions of the browser
plus other Geko based browsers.

I have not tested for any issues that may cause this to work with a required
horizontal scroll creating an unwanted vertical scroll but this should be check
for at the same time.


Reproducible: Always

Steps to Reproduce:
1.division filled with text inside a container division
2.container division has auto width heigth and overflow
3.tested against browsers listed

Actual Results:  
div/text flows under the vertical scroll bar

Expected Results:  
should flow to the left hand side of scroll bar

Comment 1

[G Evans]

Taken a bit more of a look at this and it occurs when the scroll bars are needed
on a boz div and are not to occur on the body. This means problems in trying to
implement pure CSS frames.
Also see the following forum posts
http://forums.mozillazine.org/viewtopic.php?t=315024
http://forums.mozillazine.org/viewtopic.php?t=268104
some similarities?

Comment 2

[Christian Vitroler]

Can confirm that with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5)
Gecko/20050929 Firefox/1.4. Opera 8.5 renders it correctly

Comment 3

[Martijn Wargers (dead)]

Attachment: testcase

Comment 4

[Martijn Wargers (dead)]

I think this is actually correct behavior.
But cc-ing roc to be sure. If I remember correctly, he changed this behavior.

If you don't want this to happen, just don't set overflow at all for the
containing box.

Comment 5

[G Evans]

Please note, this bug is with scrolling: auto; not scroll. The test case created
is NOT appropriate.

The issue is that the algorithem correctly asserts that a vertical scrollbar but
DOESN'T recalculate the width to include it hence the visible widow is covered
by the scrollbar. Thi is incorrect and reverse the situation of previous
versions and most other browsers. Even IE can get it right. IMHO this is quite
serious as it is a potential web page breaker.

Comment 6

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

I'll look at this but it would help if the overflow:auto testcase was minimized
a bit

Comment 7

[Martijn Wargers (dead)]

Attachment: testcase2, using overflow:auto

Yes, sorry about that, but it didn't really change the situation.
This testcase reflects what this bug is about, not?

This bug is about edge detection, which Mozilla1.7 did from inside the vertical
scrollbar, but current trunk builds do from the box edge.

Comment 8

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

Oh right.

I'm pretty sure that what we're doing is correct as per spec. The presence or
absence of scrollbars should not change the size of the absolute containing
block. Ian, can you confirm?

Comment 9

[Hixie (not reading bugmail)]

This looks like a bug to me. The scrollbar is inserted between the inner border
edge and the outer padding edge; the inner element is positioned relative to the
outer padding edge. Thus when the scrollbar is added, the inner element shrinks
a bit. Thus there should never be any need for a horizontal scrollbar. No?

Comment 10

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

Yeah I guess so. What's confusing is that the padding is inside the scrolled area.

I feel sure we had other bugs on padding and positioning in scrolled elements...

Comment 11

[G Evans]

Attachment: test case 3

I have tried to cut down the original test page and add as a test case. Hope
that works. It is a bit long as I want to force the horizontal scroll as that
shows the issue. For description of DIVs see original URL. Neither of the other
cases show it as they are too short. If it doesn't show then reduce the size of
the widow until it does. You will then see text dissapearing under the vertical
scroll bar - THAT is the issue. Doesn't do it on ANY other Gecko based browsers
I've tried and, if I hack the page for IE, even it does it right. The covered
text is the issue.

Comment 12

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

Attachment: fix

This seems to work. Really, it should be possible to get the padding-box either
from the frame or the reflow state.

Comment 13

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

This is a layout regression, and a reasonably serious one.

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 198063 [details] [diff] [review]
fix

It might be nicer to, instead of PR_MAX, do something like

if (cbSize.width < 0)
  cbSize.width = 0;

but r+sr=dbaron either way.

Comment 15

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

checked in as-is. We should really replace PR_MAX/PR_MIN with templated
functions that evaluate their arguments exactly once.

Comment 16

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

Comment on attachment 198063 [details] [diff] [review]
fix

There is some risk here --- not much, I hope --- but this is a fairly serious
layout regression.

Comment 17

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

checked in on branch.

Comment 18

[G Evans]

Comment on attachment 198060 [details]
test case 3

Sorry, my bad. I messed up the attachment. Seems irrelevent now so will not
fix. If you want a fixed one, for the recod, please ask.

Comment 19

[G Evans]

Thank you gentlemen. This looks good in the nightly build.

Comment 20

[G Evans]

Comment on attachment 198060 [details]
test case 3

><!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">
><HTML lang="en"><HEAD><TITLE>Deer Park test page</TITLE>
>  
>
>  <META http-equiv="content-type" content="text/html; charset=ISO-8859-1"/>
>  <META name="author" content="Geraint Evans"/>
>
>
>    <STYLE type="text/css">&lt;!--
>
>body {
>  height: 100%;
>  width: 100%;
>  margin: 0;
>  padding: 0;
>  overflow: hidden;
>  background-color: #0f0;
>  }
>
>
>#box {
>  position: absolute;
>  top: 0;
>  bottom: 0;
>  left: 0;
>  right: 0;
>  height: 100%;
>  width: 100%;
>  height: auto;
>  width: auto;
>  margin: 0;
>  padding: 0;
>  overflow: hidden;
>  border: none;
>  background-color: #CFC;
>  z-index: 1;
>  }
>
>
>#sbox {
>  position: absolute;
>  left: 5%;
>  right: 5%;
>  top: 70px;
>  bottom: 41px;
>  height: auto;
>  width: auto;
>  margin: 0;
>  padding: 0;
>  overflow: auto;
>  border: none;
>  background-color: #f00;
>  z-index: 2;
>  }
>
>
>#story {
>  position: absolute;
>  left: 131px;
>  right: 1px;
>  top: 1px;
>  width: auto;
>  margin: 0;
>  padding: 0;
>  border: none;
>  background-color: #CFC;
>  z-index: 3;
>  }
>
>
>
>
>p {
>  margin: 1em 0 1em 0;
>  padding: 1em 0 1em 0;
>  font-family: Trebuchet MS, Arial, Helvetica, Geneva, sans-serif;
>  font-size: 0.9em;
>  color: #030;
>  }
>
>    --&gt;</STYLE><LINK rel="STYLESHEET" href="chrome://targetalert/content/onMouseoverStyle.css"/></HEAD>
>
>
>
><BODY>
>
>
>  <DIV id="box">
>
>
>    <DIV id="sbox">
>
>
>      <DIV id="story">
>
>
>        <P>Top.</P>
>        <P>1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 </P>
>        <P>123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 </P>
>        <P>12345678 12345678 12345678 12345678 12345678 12345678 12345678 12345678 12345678 12345678 12345678 12345678 12345678 12345678 12345678 12345678 12345678 12345678 12345678 12345678 12345678 12345678 </P>
>        <P>The end.</P>
>
>
>      </DIV><!-- end of "story" -->
>
>
>    </DIV><!-- end of "sbox" -->
>
>
>  </DIV><!-- end of "box" -->
>
>  </BODY></HTML>

Comment 21

[G Evans]

Attachment: test case 4

Added the test case from 310736 as I give up on trying to edit test case 3!

Comment 22

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I'm likely to back this out because of bug 310736, in particular because of bug
310736 comment 6 (which scares me a good bit regarding incremental reflow bugs
that may take a little time to surface).

I'm guessing that this is a regression from bug 282754, although I'm not sure.

Comment 23

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Backed out of trunk and 1.8 branch.

Comment 24

[Scott MacGregor]

Comment on attachment 198063 [details] [diff] [review]
fix

clearing the approval flag for a patch that was backed out.

Comment 25

[G Evans]

Can someone test this in Linux and Mac? If they are affected the 'OS' flag needs
to be changed. I suspect these are affected but I have no facilities to test.

Comment 26

[Marian POPESCU]

Confirmed with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b5) Gecko/20051004
Firefox/1.4.1 - Build ID: 2005100404

Comment 27

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Confirmed on Linux on Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b5)
Gecko/20050928 Firefox/1.4; assuming all platforms in that case.

Comment 28

[Asa Dotzler [:asa]]

we'll have to try in RC.

Comment 29

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

Working on this, I discovered a nasty little problem ... I had assumed that the
presence or absence of a horizontal scrollbar would not affect the layout of the
descendants, but in fact it can, when there are absolutely positioned children
positioned relative to the bottom padding-edge.

Comment 30

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

Attachment: fix #2

This patch addresses the vertical scrollbar issue. It does *not* address the
horizontal scrollbar issue (e.g. testcases that use 'bottom'), for the reason
in my previous comment: that requires a somewhat deeper and riskier fix.

Comment 31

[Arthur Langereis]

I have a similar bug and I was wondering if it is related to/exactly the same as
this one:

viz: http://thuros.kicks-ass.net/~arthur/iframescroll.html

Here I have an iframe element with the scrolling attribute set to "yes".
In previous versions of Gecko, no scrollbars are drawn if they aren't necessary.
Now, however, they always appear, which is really not what I want.

Additionally, and this is referenced I think in comment #29, I have an
absolutely positioned element with bottom: 1px; This now appears BELOW the
scrollbar and causes the document to have an active vertical scrollbar.

None of this happens in FF pre-1.5, Safari or IE (well, IE always shows a
vertical scrollbar, but that is exactly why we use the scrolling="yes" attr, as
leaving it out causes similar problems in IE when iframe contents exceed the
height of the iframe element.)

Is this the same problem or do I need to file another bug?

To see this problem in a real setting, visit:
http://aqua.queenslibrary.org/

The center results iframe suffers from these problems.

I can (probably) work around these problems by special casing for Moz in the
generation of the iframe element and leaving scrolling="yes" out, but I'd rather
not add a hack when it's not necessary.

Comment 32

[Martijn Wargers (dead)]

(In reply to comment #31)
> Is this the same problem or do I need to file another bug?
This is a different issue. And it is not really a bug in Mozilla. This is
something that has been fixed in the upcoming 1.5 release, see the scrolling
attribute at:
http://www.w3.org/TR/html401/present/frames.html#h-16.2.2
"yes: This value tells the user agent to always provide scrolling devices for
the frame window."
If you still think it is a bug in Mozilla, file a new bug, but don't comment in
this bug.

Comment 33

[amano]

Hmm. What are the options here?

Fixing it just partly with Roc's current patch?
Backing out bug # 282754?
Waiting for a better fix?

Comment 34

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

We need this fix. I don't think we can do anything better for FF1.5.

Comment 35

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 198756 [details] [diff] [review]
fix #2

I really really don't like this patch, but r+sr=dbaron.

What makes this code really bad is that we're being inconsistent about whether
we want to store the state we need in the frame tree or the reflow state chain.
 I think the easier solution given where we currently are is to store it all in
the reflow state chain, i.e., store the absolute containing block information
(probably a rectangle) in the reflow state, unconditionally, so it's always
there, perhaps even using one of the members of the reflow state that we
currently ignore when reflowing an absolutely positioned frame.  In the longer
term, we're probably better relying more on the frame tree and less on reflow
states (which probably requires setting size (or at least width) information on
frames earlier in reflow).

Comment 36

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

I agree that it's inconsistent, but I don't think storing the rect in the reflow
state is immediately easier because to do it right we'd have to change code in a
few more places.

The issue that really worries me is the dependency of the layout of the children
on whether there's a horizontal scrollbar. That means I need to mash scrollframe
layout again. I think I'll put it off until the reflow branch lands, at least.

Checked into trunk.

Comment 37

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

Comment on attachment 198756 [details] [diff] [review]
fix #2

Need approval for this blocker (partial) fix.

The patch looks big but there's only a couple of meaningful changes, in
nsBlockFrame and nsGfxScrollFrame.

Comment 38

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

checked into trunk

Comment 39

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

I mean, checked into branch

Comment 40

[G Evans]

I have tried the nightly build and the problem seems to have been fixed as well
as 310736. However, when I shrink down to 640x480 I am seeing a 1px background
line next to the LHS of the vertical scroll bar. Rounding error or side effect?

Comment 41

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

This is fixed. G Evans, what testcase is that?

Comment 42

[G Evans]

(In reply to comment #41)
> This is fixed. G Evans, what testcase is that?

Rob

Testcase 4 shows it. It has a deliberate 1px border to show the RHS for the
original testing but if you shrink it, bit by bit, a second pixel shows, ie the
border varies between 1px and 2px in width. This looks like a rounding issue as
it is right in some sizes but not others. You do need to shrink it until the
vertical scroll bar appears as it does not show until then. The web page I am
building does not have a border but has the same effect.

Comment 43

[Martijn Wargers (dead)]

G Evans, I'm seeing the bug too, could you file a new bug on that and mention the bug number here?

Comment 44

[G Evans]

(In reply to comment #43)
> G Evans, I'm seeing the bug too, could you file a new bug on that and mention
> the bug number here?
> 

Have added it as bug 313543. Not sure if I have done the dependency correctly (newbie). Can you vote/confirm please so we can get the ball rolling.