Bug 14094 – REGRESSION: Issues with scrollbars in phpbb2 based forums

• Component: Layout and Rendering
• Status: RESOLVED WONTFIX

[Adam Williams]

in "Safari 3.0":
phpbb2 forums have scroll bars in about 50% of posts for no real reason, there
is no content to scroll but for some reason scroll bars are appearing in these
areas.
example:
http://forum.ytunnelpro.com/viewtopic.php?t=745

Unsure if the phpbb2 problem was new safari or webkit so it was also filed with
bugreports.apple.com under:
"5263028         Scroll bars appearing where they are not needed."

[David Kilzer (:ddkilzer)]

Confirmed with a local debug build of WebKit r22089 with shipping Safari 2.0.4 (419.3) on Mac OS X 10.4.9 (8P135).

This is also a regression from shipping Safari.

[David Kilzer (:ddkilzer)]

If I would only read closer:

<rdar://problem/5263028>

[Adam Williams]

Just thought I'd add note the problem still persists with webkit 522.12 and Safari 3.0.2 update. Although I figured it would but just covering all bases :)

[David Kilzer (:ddkilzer)]

Created test account (waiting to be approved):
E: [e-mail]
P: w3bk1t

[David Kilzer (:ddkilzer)]

(In reply to comment #4)
> Created test account (waiting to be approved):
> E: [e-mail]
> P: w3bk1t

U: webkit

Created attachment 16184 [details]
Reduction

This reduction shows that the scrollbar appears because of small line-height. Firefox renders a scrollbar, too, in this case, but Opera does not.

The reason this is a regression for the live site, I suspect, is that the overflow rule is inside a media query, which Safari 2 doesn't support. The media query used, "@media all and (min-width: 0px)", is a catch-all, so I think it is just a lame Opera detector (see bug 13980).

[David Kilzer (:ddkilzer)]

Verified that removing this CSS from http://forum.ytunnelpro.com/templates/ca_aphrodite/style.css removes the unwanted scrollbars on the site:

@media all and (min-width: 0px) {
        html>body .post-text {
                overflow: auto;
        }
}       

[David Kilzer (:ddkilzer)]

Also confirmed that Safari 2.0.4 (419.3) with original WebKit on Mac OS X 10.4.10 (8R218) behaves the same way if the CSS is present without being enclosed by a media query, so the behavior in the reduction isn't really a regression.

The only regression is the way the web site behaves because WebKit now understands @media queries.

[Adam Williams]

Created attachment 17457 [details]
Screen Captures

Screen captures of site behavior in Safari 3.0.4 with correct site behavior and same identical site with same user/password and url on webkit nightly r27953 showing site non functional

[Adam Williams]

Comment on attachment 17457 [details]
Screen Captures

oops my bad. I was trying to upload this to an entirely different bug report. Completely Ignore this file.

[Benjamin Poulain]

Created attachment 70949 [details]
Layout test for this problem

[Benjamin Poulain]

Created attachment 71054 [details]
Other interesting use case of the same problem

This could be added as a second layout test. It shows the layout of inline flow + floating boxes.

[Benjamin Poulain]

Created attachment 71060 [details]
First attempt to solve the issue

I am not very familiar with the layout of inline flow so input on the patch would be appreciated :)

One of my concern was the layout of floating elements inside the block after text with line-height < font-size. Our current layout is consistent with Internet Explorer so I did not pushed my change further than at the end of the layout of the inline flow.

This will break quite a few layout test since the size of block is bigger than before when line-height < font-size.

[Benjamin Poulain]

Created attachment 71061 [details]
It's better with the patch :)

[Benjamin Poulain]

Comment on attachment 71061 [details]
It's better with the patch :)

Removing from the review queue. This patch breaks the test css2.1/t0804-c5510-ipadn-00-b-ag.html and should not.

[Benjamin Poulain]

Created attachment 71297 [details]
Patch: define the lowest position as the position of the block

I was trying to avoid this solution because I did not want the text to be clipped. However, not clipping the text in this case would not follow the CSS specification regarding the size of the block.

Internet explorer is also clipping the text at the bottom in this case.

[Benjamin Poulain]

Anything I can do to help getting a review? You need more explanation about the patch?

With the patch, the following tests will need to be rebased:
mathml/presentation/fenced.xhtml
mathml/presentation/mo.xhtml
mathml/presentation/over.xhtml
mathml/presentation/row-alignment.xhtml
mathml/presentation/row.xhtml

I have checked the new results and they still look fine. The differences comes from layers being smaller with this change.

[Kenneth Rohde Christiansen]

Hyatt, could you have a look at this one?

[Dave Hyatt]

Yeah this issue occurred to me when I was working on the rewrite recently.  We need to be using the line-height box rather than the content box for lines when considering overflow.  Now that layout overflow and visual overflow are truly separated this should be pretty easy to fix.

All you really have to do here is change the default rectangle for layout overflow from the line box's x/y/width/height to a box that includes the line height.  (Remember to account for vertical text and expand/shrink the appropriate axis).

The lineTop/lineBottom clamping would also have to change to factor in line-height.

[Dave Hyatt]

I'm not completely convinced we should fix this though.  I like that the text is reachable.  I'd only consider fixing this if it's a major compatibility issue.

[Dave Hyatt]

Comment on attachment 71297 [details]
Patch: define the lowest position as the position of the block

Minusing since this code all got changed.

[Benjamin Poulain]

(In reply to comment #20)
> I'm not completely convinced we should fix this though.  I like that the text is reachable.  I'd only consider fixing this if it's a major compatibility issue.

I noticed this problem is quite common on code examples. For some reason web developers tend to set line-height < font-size there (e.g. http://developer.qt.nokia.com/forums/viewthread/1184/ ).

I remember seeing this on http://developer.apple.com/ some time ago. And that worked well with IE9. I cannot find it anymore so I guess the css have changed.

Sure you don't want a fix? We close the bug as wontfix?

[Dave Hyatt]

What bugs me about it is that when content spills out of a regular block, we make it reachable via overflow mechanisms.  I don't see why content that spills out of a line box should be any different.  There is actual content that the user might want to see.  Clipping the text certainly doesn't seem desirable.

Basically I'm inclined to say WONTFIX unless the compatibility problem is severe enough that it looks like we need to do it.  I'd be curious to know if we're the only browser engine doing this and if all the others clip the text.

[Benjamin Poulain]

(In reply to comment #23)
> What bugs me about it is that when content spills out of a regular block, we make it reachable via overflow mechanisms.  I don't see why content that spills out of a line box should be any different.  There is actual content that the user might want to see.  Clipping the text certainly doesn't seem desirable.

What is counterintuitive to me is that "overflow: visible" produce scrollbar in that case.

> Basically I'm inclined to say WONTFIX unless the compatibility problem is severe enough that it looks like we need to do it.  

Fine with me, I close the task. I guess I still have learned something on the way :)

> I'd be curious to know if we're the only browser engine doing this and if all the others clip the text.

Firefox and Opera behave as WebKit in this case. Internet Explorer clip the block.