Bug 11926 - <div> tag causes the horizontal scrollbars display
Summary: <div> tag causes the horizontal scrollbars display
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Dave Hyatt
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2006-12-22 06:20 PST by Madhu M
Modified: 2007-05-10 16:17 PDT (History)
3 users (show)

See Also:


Attachments
Sample html showing the bug (479 bytes, text/html)
2006-12-22 06:24 PST, Madhu M
no flags Details
patch for this issue (4.41 KB, patch)
2007-03-26 18:28 PDT, Madhu M
hyatt: review-
Details | Formatted Diff | Diff
patch for this issue (13.99 KB, patch)
2007-04-02 17:51 PDT, Madhu M
hyatt: review-
Details | Formatted Diff | Diff
Patch to fix this bug. (2.52 KB, patch)
2007-05-10 02:00 PDT, Dave Hyatt
oliver: review+
Details | Formatted Diff | Diff
RTL case broken after the fix (363 bytes, text/html)
2007-05-10 02:26 PDT, mitz
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Madhu M 2006-12-22 06:20:30 PST
<html>
<body>
<div id="div1" style="top:-800;position:absolute;overflow:hidden"><xmp><span id="span1"><table><tr><td>&nbsp;</td></tr><tr><td>Unauthorized Overtime</td></tr><tr><td>Incorrect Assignment</td></tr><tr><td>Error</td></tr><tr><td>IECPP: Incorrect Account Assignment</td></tr><tr><td>Wrong Activity type</td></tr><tr><td >Wrong Order</td></tr><tr><td >Wrong A/A Type</td></tr></table></span></xmp></div>
</body>
</html>

The attached html if opened in Safari causes the Horizontal scroll bar to get displayed. But in Firefox it is not comming. This html is having an <div> tag and an <xmp> inside
Comment 1 Madhu M 2006-12-22 06:24:00 PST
Created attachment 11963 [details]
Sample html showing the bug

When the file is opened in Safari it causes the horizontal scroll bar to get displayed. There is nothing on the web page to be displayed, but scroll bar comes. In firefox the behaviour is proper, ie no scroll bar is displayed
Comment 2 David Kilzer (:ddkilzer) 2006-12-22 06:42:36 PST
This looks correct to me.  The "top:-800;" style attribute of the DIV makes the content of the XMP tag "invisibile", but if you remove that, you'll see that the horizontal scrollbar is needed to view all of the content.

Comment 3 David Kilzer (:ddkilzer) 2006-12-22 06:52:28 PST
(In reply to comment #2)
> This looks correct to me.  The "top:-800;" style attribute of the DIV makes the
> content of the XMP tag "invisibile", but if you remove that, you'll see that
> the horizontal scrollbar is needed to view all of the content.

It also makes it "invisible"!

Comment 4 Jake Logan 2006-12-22 11:31:16 PST
(In reply to comment #3)
> (In reply to comment #2)
> > This looks correct to me.  The "top:-800;" style attribute of the DIV makes the
> > content of the XMP tag "invisibile", but if you remove that, you'll see that
> > the horizontal scrollbar is needed to view all of the content.
> 
> It also makes it "invisible"!
> 

Hi David,

I do understand that if you remove the "top:-800;" that will make the content of the DIV visible. However, the purpose of this request is to cause the content of the <XMP> to be *invisible* and to take 0 pixels of space. The XMP tag is used to load code dynamically and the intention is for that code to be invisible and not affect the layout of the DIV at all. We can't change that code and it works as I describe in Firefox and IE. 

Can you please reconsider? This issue causes erroneous nested scrollbars to appear in Safari in many SAP installations, including some important Apple customers. It is one of the key issues that needs to be fixed for SAP/Safari compatibility.

Best,

Jake

Comment 5 David Kilzer (:ddkilzer) 2006-12-22 12:05:57 PST
> Can you please reconsider? This issue causes erroneous nested scrollbars to
> appear in Safari in many SAP installations, including some important Apple
> customers. It is one of the key issues that needs to be fixed for SAP/Safari
> compatibility.

Interesting.  I was just reporting what I found in a brief analysis of the HTML.  (My CSS analysis needs some work!)  After reviewing the CSS, should the "overflow:hidden" property be clipping the content that is not visible, and thus not drawing a scrollbar?

Confirming bug--test case works as expected in Firefox 1.5.0.9 (on WinXP).
Comment 6 Jake Logan 2006-12-22 12:31:35 PST
(In reply to comment #5)
> > Can you please reconsider? This issue causes erroneous nested scrollbars to
> > appear in Safari in many SAP installations, including some important Apple
> > customers. It is one of the key issues that needs to be fixed for SAP/Safari
> > compatibility.
> 
> Interesting.  I was just reporting what I found in a brief analysis of the
> HTML.  (My CSS analysis needs some work!)  After reviewing the CSS, should the
> "overflow:hidden" property be clipping the content that is not visible, and
> thus not drawing a scrollbar?
> 
> Confirming bug--test case works as expected in Firefox 1.5.0.9 (on WinXP).
> 


Yes, I believe that the "overflow:hidden" should cause the content to be clipped (or hidden) and not show scrollbars, as described here:

http://www.w3schools.com/css/pr_pos_overflow.asp

and here:

http://www.quirksmode.org/css/overflow.html

I'm no CSS expert either, but the behavior we see in Firefox (both Mac and Windows) I believe to be correct.

Thanks for your thoughts.

Jake
Comment 7 Dave Hyatt 2006-12-22 13:51:19 PST
Yeah this looks like a bug.
Comment 8 Alice Liu 2007-01-31 15:42:49 PST
<rdar://problem/4694859>
Comment 9 Madhu M 2007-03-23 16:45:14 PDT
I have got the reason for this isuue in Webkit. It is while calculating thr right most position in RenderBolck
(RenderBlock::rightmostPosition) for positined objects. Here the overflow clip is not considered and the check is done only for the objects having position:Fixed.  There is a comment in code like :-

 " Fixed positioned objects do not scroll and thus should not constitute part of the rightmost position" (?).

I think overflow:hidden is to be considered here. When I give check with hasOverflowClip() function, it will ignore that object also while calculationg width and right most position. I think it is a valid fix for the issue.

Regards

Madhu
Comment 10 Madhu M 2007-03-26 18:28:22 PDT
Created attachment 13823 [details]
patch for this issue

I am submitting the patch for this issue. The scroll bars are getting displayed because of  wrong width calculation. For calculating the width, the right most position is used. But it is not considering the overflow clip, while checking for rightmost position.

The patch has a layout test case. This patch is not creating any failures in the existing layout tests.
Comment 11 Dave Hyatt 2007-03-26 19:11:20 PDT
Comment on attachment 13823 [details]
patch for this issue

This is not the right fix.  rightmostPosition should have returned the correct answer.  You also didn't patch lowestPosition or leftmostPosition (which presumably suffer from the same issue).
Comment 12 Dave Hyatt 2007-03-26 19:13:39 PDT
Comment on attachment 13823 [details]
patch for this issue

(The reason this isn't right is you need to at least consider the positioned object's border box, even when overflow is not set to visible.)
Comment 13 Dave Hyatt 2007-03-26 20:08:17 PDT
The real issue here is that content that cannot be scrolled to in one direction (e.g., vertically) should not be allowed to affect a different axis (horizontal).
Comment 14 Dave Hyatt 2007-03-26 20:09:06 PDT
This basically includes content positioned entirely "above" or "to the left" of the scrollable area.
Comment 15 Madhu M 2007-03-27 15:34:14 PDT
Hi Dave Hyatt, 

Thanks for your comments. 

From your comments I have made the following points 

1. Along with rightmost position we have to make similar checks for lowestPosition or leftmostPosition
2. If the overflow is given as hidden we need to consider the render box of thepositioned objects to ensure that overflow is clipped in both the directions.

3. Contents which cannot be scrolled in horizontal direction should not be allowed to scroll vertically.

4. We need to clip the items positioned above or left of the scrollable area.

Are they correct. Please give your suggestions.

Regards

Madhu
Comment 16 Madhu M 2007-04-02 17:51:37 PDT
Created attachment 13930 [details]
patch for this issue

patch containing fix for the overflow:hidden issue in WebKit
Comment 17 Madhu M 2007-04-02 17:52:55 PDT
Comment on attachment 13930 [details]
patch for this issue

I have made the changes according to the comments given by Dave Hyatt. I have made the check for overflow clip while calculating rightmost and leftmost positions.

In Firefox it is allowing the vertical scroll even if the overflow is set to hidden. So there is no need to check for the lowermost position.

Now the behavior in Safari is made similar to that in Firefox. I have added two new test cases for this. Please review my patch and give the comments.

Regards

Madhu
Comment 18 Darin Adler 2007-04-05 09:17:10 PDT
Comment on attachment 13930 [details]
patch for this issue

The change logs in this patch are not good. There are double entries in each change log, and it looks like the output of the prepare-ChangeLog without any editing. That script is supposed to start a change log for you, but then you have to add the appropriate comments to explain what you're changing and why.
Comment 19 Dave Hyatt 2007-04-05 11:44:29 PDT
Comment on attachment 13930 [details]
patch for this issue

This is not correct.  It will break Web sites.

As I said before, the real fix is pretty complicated, since you have to detect that an object is entirely above or to the left of the scrollable area in order to exclude it.
Comment 20 Dave Hyatt 2007-05-10 02:00:43 PDT
Created attachment 14454 [details]
Patch to fix this bug.

Will land test case as layout test.
Comment 21 Oliver Hunt 2007-05-10 02:03:54 PDT
Comment on attachment 14454 [details]
Patch to fix this bug.

ChangeLog, and testcase if possible
Comment 22 mitz 2007-05-10 02:06:39 PDT
+                // If a positioned object lies completely to the left of our object it will be unreachable via scrolling.
+                // Therefore we should not allow it to contribute to the lowest position.

Is this correct for RTL overflows?
Comment 23 Dave Hyatt 2007-05-10 02:15:02 PDT
Fixed in r21358
Comment 24 mitz 2007-05-10 02:26:08 PDT
Created attachment 14455 [details]
RTL case broken after the fix

RTL should have a vertical scroll bar too.
Comment 25 David Kilzer (:ddkilzer) 2007-05-10 16:17:08 PDT
(In reply to comment #24)
> Created an attachment (id=14455) [edit]
> RTL case broken after the fix
> 
> RTL should have a vertical scroll bar too.

Fixed in r21360 with layout test in r21361.