overflow:hidden fixes for scrollLeft/Top/Width/Height

David Hyatt hyatt at apple.com
Tue Oct 7 14:21:20 CEST 2003


Index: ChangeLog
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/ChangeLog,v
retrieving revision 1.2060
diff -u -p -r1.2060 ChangeLog
--- ChangeLog	2003/10/07 04:43:23	1.2060
+++ ChangeLog	2003/10/07 07:17:39
@@ -1,3 +1,44 @@
+2003-10-07  David Hyatt  <hyatt at apple.com>
+
+	Fix for 3363421, event handlers could be triggered for content outside an overflow:hidden
+	area.  The layer checks that test for intersection/point containment need to only include
+	layers with overhanging floats if the element is overflow:visible.
+
+	Fix for 3366801, assignment to scrollLeft/Top of an overflow:hidden layer makes the layer
+	disappear.  overflow:hidden blocks actually were never computing their scroll dimensions,
+	and so had bogus answers for those values.
+
+	Fix for 3366686, no reliable scrollHeight/Width reporting for overflow:hidden or
+	overflow:visible elements.  The former was caused by the same bug as 3366801.  The
+	latter was just me using the wrong method (clientWidth/Height instead of
+	overflowWidth/Height).
+
+	This patch also tightens the assignment to scrollLeft/Top to not do anything if you don't 
+	have an overflow value other than visible.
+	
+        Reviewed by darin
+
+        * khtml/ecma/kjs_dom.cpp:
+        (DOMNode::putValue):
+        * khtml/rendering/render_block.cpp:
+        (khtml::RenderBlock::layoutBlock):
+        * khtml/rendering/render_flexbox.cpp:
+        (khtml::RenderFlexibleBox::layoutBlock):
+        * khtml/rendering/render_layer.cpp:
+        (RenderLayer::RenderLayer):
+        (RenderLayer::scrollToOffset):
+        (RenderLayer::scrollWidth):
+        (RenderLayer::scrollHeight):
+        (RenderLayer::computeScrollDimensions):
+        (RenderLayer::updateScrollInfoAfterLayout):
+        (RenderLayer::intersectsDamageRect):
+        (RenderLayer::containsPoint):
+        * khtml/rendering/render_layer.h:
+        * khtml/rendering/render_object.cpp:
+        (RenderObject::scrollWidth):
+        (RenderObject::scrollHeight):
+        * khtml/rendering/render_object.h:
+
 2003-10-06  David Hyatt  <hyatt at apple.com>
 
 	Several fixes preparing for the incremental repainting patch to be enabled.
Index: khtml/ecma/kjs_dom.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/ecma/kjs_dom.cpp,v
retrieving revision 1.34
diff -u -p -r1.34 khtml/ecma/kjs_dom.cpp
--- khtml/ecma/kjs_dom.cpp	2003/09/19 23:55:42	1.34
+++ khtml/ecma/kjs_dom.cpp	2003/10/07 07:17:45
@@ -367,13 +367,13 @@ void DOMNode::putValue(ExecState *exec, 
     break;
   case ScrollTop: {
     khtml::RenderObject *rend = node.handle() ? node.handle()->renderer() : 0L;
-    if (rend && rend->layer())
+    if (rend && rend->layer() && rend->style()->hidesOverflow())
         rend->layer()->scrollToYOffset(value.toInt32(exec));
     break;
   }
   case ScrollLeft: {
     khtml::RenderObject *rend = node.handle() ? node.handle()->renderer() : 0L;
-    if (rend && rend->layer())
+    if (rend && rend->layer() && rend->style()->hidesOverflow())
       rend->layer()->scrollToXOffset(value.toInt32(exec));
     break;
   }
Index: khtml/rendering/render_block.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/rendering/render_block.cpp,v
retrieving revision 1.63
diff -u -p -r1.63 khtml/rendering/render_block.cpp
--- khtml/rendering/render_block.cpp	2003/10/07 04:43:23	1.63
+++ khtml/rendering/render_block.cpp	2003/10/07 07:17:54
@@ -530,10 +530,10 @@ void RenderBlock::layoutBlock(bool relay
     if (m_overflowHeight < m_height)
         m_overflowHeight = m_height;
     
-    // Update our scrollbars if we're overflow:auto/scroll now that we know if
+    // Update our scroll information if we're overflow:auto/scroll/hidden now that we know if
     // we overflow or not.
-    if (style()->scrollsOverflow() && m_layer)
-        m_layer->checkScrollbarsAfterLayout();
+    if (style()->hidesOverflow() && m_layer)
+        m_layer->updateScrollInfoAfterLayout();
 
 #ifdef INCREMENTAL_REPAINTING
     // Repaint with our new bounds if they are different from our old bounds.
Index: khtml/rendering/render_flexbox.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/rendering/render_flexbox.cpp,v
retrieving revision 1.5
diff -u -p -r1.5 khtml/rendering/render_flexbox.cpp
--- khtml/rendering/render_flexbox.cpp	2003/10/07 04:43:23	1.5
+++ khtml/rendering/render_flexbox.cpp	2003/10/07 07:17:55
@@ -322,10 +322,10 @@ void RenderFlexibleBox::layoutBlock(bool
     if (m_overflowWidth < m_width)
         m_overflowWidth = m_width;
 
-    // Update our scrollbars if we're overflow:auto/scroll now that we know if
+    // Update our scrollbars if we're overflow:auto/scroll/hidden now that we know if
     // we overflow or not.
-    if (style()->scrollsOverflow() && m_layer)
-        m_layer->checkScrollbarsAfterLayout();
+    if (style()->hidesOverflow() && m_layer)
+        m_layer->updateScrollInfoAfterLayout();
 
 #ifdef INCREMENTAL_REPAINTING
     // Repaint with our new bounds if they are different from our old bounds.
Index: khtml/rendering/render_layer.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/rendering/render_layer.cpp,v
retrieving revision 1.53
diff -u -p -r1.53 khtml/rendering/render_layer.cpp
--- khtml/rendering/render_layer.cpp	2003/10/07 04:43:23	1.53
+++ khtml/rendering/render_layer.cpp	2003/10/07 07:17:56
@@ -88,6 +88,7 @@ m_scrollX( 0 ),
 m_scrollY( 0 ),
 m_scrollWidth( 0 ),
 m_scrollHeight( 0 ),
+m_scrollDimensionsDirty( true ),
 m_hBar( 0 ),
 m_vBar( 0 ),
 m_scrollMediator( 0 ),
@@ -424,8 +425,12 @@ RenderLayer::scrollToOffset(int x, int y
 {
     if (x < 0) x = 0;
     if (y < 0) y = 0;
-    int maxX = m_scrollWidth - m_object->clientWidth();
-    int maxY = m_scrollHeight - m_object->clientHeight();
+
+    // Call the scrollWidth/Height functions so that the dimensions will be computed if they need
+    // to be (for overflow:hidden blocks).
+    int maxX = scrollWidth() - m_object->clientWidth();
+    int maxY = scrollHeight() - m_object->clientHeight();
+    
     if (x > maxX) x = maxX;
     if (y > maxY) y = maxY;
 
@@ -561,9 +566,23 @@ RenderLayer::positionScrollbars(const QR
 #define LINE_STEP   10
 #define PAGE_KEEP   40
 
-void
-RenderLayer::checkScrollbarsAfterLayout()
+short RenderLayer::scrollWidth()
+{
+    if (m_scrollDimensionsDirty)
+        computeScrollDimensions();
+    return m_scrollWidth;
+}
+
+int RenderLayer::scrollHeight()
+{
+    if (m_scrollDimensionsDirty)
+        computeScrollDimensions();
+    return m_scrollHeight;
+}
+
+void RenderLayer::computeScrollDimensions(bool* needHBar, bool* needVBar)
 {
+    m_scrollDimensionsDirty = false;
     int rightPos = m_object->rightmostPosition();
     int bottomPos = m_object->lowestPosition();
 
@@ -571,15 +590,28 @@ RenderLayer::checkScrollbarsAfterLayout(
     int clientHeight = m_object->clientHeight();
     m_scrollWidth = clientWidth;
     m_scrollHeight = clientHeight;
-    
+
     if (rightPos - m_object->borderLeft() > m_scrollWidth)
         m_scrollWidth = rightPos - m_object->borderLeft();
     if (bottomPos - m_object->borderTop() > m_scrollHeight)
         m_scrollHeight = bottomPos - m_object->borderTop();
-    
-    bool needHorizontalBar = rightPos > m_object->overflowWidth(false);
-    bool needVerticalBar = bottomPos > m_object->overflowHeight(false);
 
+    if (needHBar)
+        *needHBar = rightPos > m_object->overflowWidth(false);
+    if (needVBar)
+        *needVBar = bottomPos > m_object->overflowHeight(false);
+}
+
+void
+RenderLayer::updateScrollInfoAfterLayout()
+{
+    m_scrollDimensionsDirty = true;
+    if (m_object->style()->overflow() == OHIDDEN)
+        return; // All we had to do was dirty.
+
+    bool needHorizontalBar, needVerticalBar;
+    computeScrollDimensions(&needHorizontalBar, &needVerticalBar);
+    
     bool haveHorizontalBar = m_hBar;
     bool haveVerticalBar = m_vBar;
 
@@ -606,6 +638,7 @@ RenderLayer::checkScrollbarsAfterLayout(
 
     // Set up the range (and page step/line step).
     if (m_hBar) {
+        int clientWidth = m_object->clientWidth();
         int pageStep = (clientWidth-PAGE_KEEP);
         if (pageStep < 0) pageStep = clientWidth;
         m_hBar->setSteps(LINE_STEP, pageStep);
@@ -616,6 +649,7 @@ RenderLayer::checkScrollbarsAfterLayout(
 #endif
     }
     if (m_vBar) {
+        int clientHeight = m_object->clientHeight();
         int pageStep = (clientHeight-PAGE_KEEP);
         if (pageStep < 0) pageStep = clientHeight;
         m_vBar->setSteps(LINE_STEP, pageStep);
@@ -931,7 +965,7 @@ void RenderLayer::calculateRects(const R
 bool RenderLayer::intersectsDamageRect(const QRect& layerBounds, const QRect& damageRect) const
 {
     return (renderer()->isCanvas() || renderer()->isRoot() || renderer()->isBody() ||
-            renderer()->hasOverhangingFloats() ||
+            (renderer()->hasOverhangingFloats() && !renderer()->style()->hidesOverflow()) ||
             (renderer()->isInline() && !renderer()->isReplaced()) ||
             layerBounds.intersects(damageRect));
 }
@@ -939,7 +973,7 @@ bool RenderLayer::intersectsDamageRect(c
 bool RenderLayer::containsPoint(int x, int y, const QRect& damageRect) const
 {
     return (renderer()->isCanvas() || renderer()->isRoot() || renderer()->isBody() ||
-            renderer()->hasOverhangingFloats() ||
+            (renderer()->hasOverhangingFloats() && !renderer()->style()->hidesOverflow()) ||
             (renderer()->isInline() && !renderer()->isReplaced()) ||
             damageRect.contains(x, y));
 }
Index: khtml/rendering/render_layer.h
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/rendering/render_layer.h,v
retrieving revision 1.29
diff -u -p -r1.29 khtml/rendering/render_layer.h
--- khtml/rendering/render_layer.h	2003/10/07 04:43:23	1.29
+++ khtml/rendering/render_layer.h	2003/10/07 07:17:57
@@ -120,9 +120,9 @@ public:
 
     void setWidth(short w) { m_width = w; }
     void setHeight(int h) { m_height = h; }
-    
-    short scrollWidth() const { return m_scrollWidth; }
-    int scrollHeight() const { return m_scrollHeight; }
+
+    short scrollWidth();
+    int scrollHeight();
     
     void setPos( int xPos, int yPos ) {
         m_x = xPos;
@@ -148,7 +148,7 @@ public:
 #ifdef APPLE_CHANGES
     void paintScrollbars(QPainter* p, const QRect& damageRect);
 #endif
-    void checkScrollbarsAfterLayout();
+    void updateScrollInfoAfterLayout();
     void slotValueChanged(int);
     void updateScrollPositionFromScrollbars();
 
@@ -228,6 +228,8 @@ private:
     RenderLayer* nodeAtPointForLayer(RenderLayer* rootLayer, RenderObject::NodeInfo& info,
                                      int x, int y, const QRect& hitTestRect);
 
+    void computeScrollDimensions(bool* needHBar = 0, bool* needVBar = 0);
+    
 protected:   
     RenderObject* m_object;
     
@@ -262,6 +264,7 @@ protected:   
     // The width/height of our scrolled area.
     short m_scrollWidth;
     int m_scrollHeight;
+    bool m_scrollDimensionsDirty;
     
     // For layers with overflow, we have a pair of scrollbars.
     QScrollBar* m_hBar;
Index: khtml/rendering/render_object.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/rendering/render_object.cpp,v
retrieving revision 1.102
diff -u -p -r1.102 khtml/rendering/render_object.cpp
--- khtml/rendering/render_object.cpp	2003/10/07 04:43:23	1.102
+++ khtml/rendering/render_object.cpp	2003/10/07 07:17:59
@@ -385,7 +385,7 @@ RenderObject::clientWidth() const
         (layer() ? layer()->verticalScrollbarWidth() : 0);
 }
 
-short
+int
 RenderObject::clientHeight() const
 {
     return height() - borderTop() - borderBottom() -
@@ -397,13 +397,13 @@ RenderObject::clientHeight() const
 short
 RenderObject::scrollWidth() const
 {
-    return (style()->hidesOverflow() && layer()) ? layer()->scrollWidth() : clientWidth();
+    return (style()->hidesOverflow() && layer()) ? layer()->scrollWidth() : overflowWidth();
 }
 
-short
+int
 RenderObject::scrollHeight() const
 {
-    return (style()->hidesOverflow() && layer()) ? layer()->scrollHeight() : clientHeight();
+    return (style()->hidesOverflow() && layer()) ? layer()->scrollHeight() : overflowHeight();
 }
 
 bool
Index: khtml/rendering/render_object.h
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/rendering/render_object.h,v
retrieving revision 1.83
diff -u -p -r1.83 khtml/rendering/render_object.h
--- khtml/rendering/render_object.h	2003/10/07 04:43:23	1.83
+++ khtml/rendering/render_object.h	2003/10/07 07:18:00
@@ -486,12 +486,12 @@ public:
     // More IE extensions.  clientWidth and clientHeight represent the interior of an object
     // excluding border and scrollbar.
     short clientWidth() const;
-    short clientHeight() const;
+    int clientHeight() const;
 
     // scrollWidth/scrollHeight will be the same as clientWidth/clientHeight unless the
     // object has overflow:hidden/scroll/auto specified and also has overflow.
     short scrollWidth() const;
-    short scrollHeight() const;
+    int scrollHeight() const;
 
     // The following seven functions are used to implement collapsing margins.
     // All objects know their maximal positive and negative margins.  The
-------------- next part --------------


The URLs that demo the problems are available at:

http://x.isomorphic.com/Safari_Tests/scrollWidth_scrollHeight.html
http://x.isomorphic.com/Safari_Tests/assignToScrollLeft.html
http://x.isomorphic.com/Safari_Tests/Overflow_Hidden_Events.html

These should merge easily into your current tree.

dave


More information about the Khtml-devel mailing list