Changeset 96152 in webkit

Timestamp:

Sep 27, 2011 1:55:00 PM (13 years ago)

Author:

ojan@chromium.org

Message:

offsetTop/offsetLeft return the wrong values for horizontal-bt/vertical-rl writing modes
​https://bugs.webkit.org/show_bug.cgi?id=68304

Reviewed by David Hyatt.

Source/WebCore:

When grabbing the x/y values of the RenderBox, we need to take writing mode
flipping into account.

Test: fast/dom/offset-position-writing-modes.html

• rendering/RenderBox.cpp:

(WebCore::RenderBox::locationIncludingFlipping):

• rendering/RenderBox.h:

(WebCore::RenderBox::yFlippedForWritingMode):
(WebCore::RenderBox::xFlippedForWritingMode):

• rendering/RenderBoxModelObject.cpp:

(WebCore::RenderBoxModelObject::offsetLeft):
(WebCore::RenderBoxModelObject::offsetTop):

LayoutTests:

• css3/flexbox/writing-modes-expected.txt:
• css3/flexbox/writing-modes.html:
• fast/dom/offset-position-writing-modes-expected.txt: Added.
• fast/dom/offset-position-writing-modes.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/css3/flexbox/writing-modes-expected.txt (modified) (1 diff)
• LayoutTests/css3/flexbox/writing-modes.html (modified) (2 diffs)
• LayoutTests/fast/dom/offset-position-writing-modes-expected.txt (added)
• LayoutTests/fast/dom/offset-position-writing-modes.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderBox.cpp (modified) (2 diffs)
• Source/WebCore/rendering/RenderBox.h (modified) (2 diffs)
• Source/WebCore/rendering/RenderBoxModelObject.cpp (modified) (4 diffs)
• Source/WebCore/rendering/RenderLayer.cpp (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2011-09-27  Ojan Vafai  <ojan@chromium.org>
+
+        offsetTop/offsetLeft return the wrong values for horizontal-bt/vertical-rl writing modes
+        https://bugs.webkit.org/show_bug.cgi?id=68304
+
+        Reviewed by David Hyatt.
+
+        * css3/flexbox/writing-modes-expected.txt:
+        * css3/flexbox/writing-modes.html:
+        * fast/dom/offset-position-writing-modes-expected.txt: Added.
+        * fast/dom/offset-position-writing-modes.html: Added.
+
 2011-09-27  Tim Horton  <timothy_horton@apple.com>
 

trunk/LayoutTests/css3/flexbox/writing-modes-expected.txt

@@ -13 +13 @@
 PASS
 PASS
-Expected 580 for offsetLeft, but got 0. Expected 580 for offsetLeft, but got 0. Expected 580 for offsetLeft, but got 0.
-Expected 580 for offsetLeft, but got 0. Expected 180 for offsetTop, but got 0. Expected 580 for offsetLeft, but got 150. Expected 180 for offsetTop, but got 0. Expected 580 for offsetLeft, but got 450. Expected 180 for offsetTop, but got 0.
+PASS
+PASS
 

trunk/LayoutTests/css3/flexbox/writing-modes.html

@@ -159 +159 @@
 </div>
 
-<!-- FIXME: There's a bug where offsetLeft reports the wrong value in vertical-rl writing-mode.-->
 <div style="position:relative">
 <div class="flexbox vertical-rl">
@@ -168 +167 @@
 </div>
 
-<!-- FIXME: There's a bug where offsetTop reports the wrong value in horizontal-bt writing-mode.-->
 <div style="position:relative">
 <div class="flexbox bt" style="height:200px">
-  <div data-offset-y="180" data-offset-x="580" style="width: -webkit-flex(1 0 0);"></div>
-  <div data-offset-y="180" data-offset-x="580" style="width: -webkit-flex(2 0 0);"></div>
-  <div data-offset-y="180" data-offset-x="580" style="width: -webkit-flex(1 0 0);"></div>
+  <div data-expected-width="150" data-offset-y="180" data-offset-x="0" style="width: -webkit-flex(1 0 0);"></div>
+  <div data-expected-width="300" data-offset-y="180" data-offset-x="150" style="width: -webkit-flex(2 0 0);"></div>
+  <div data-expected-width="150" data-offset-y="180" data-offset-x="450" style="width: -webkit-flex(1 0 0);"></div>
 </div>
 </div>

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-09-27  Ojan Vafai  <ojan@chromium.org>
+
+        offsetTop/offsetLeft return the wrong values for horizontal-bt/vertical-rl writing modes
+        https://bugs.webkit.org/show_bug.cgi?id=68304
+
+        Reviewed by David Hyatt.
+
+        When grabbing the x/y values of the RenderBox, we need to take writing mode
+        flipping into account.
+
+        Test: fast/dom/offset-position-writing-modes.html
+
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::locationIncludingFlipping):
+        * rendering/RenderBox.h:
+        (WebCore::RenderBox::yFlippedForWritingMode):
+        (WebCore::RenderBox::xFlippedForWritingMode):
+        * rendering/RenderBoxModelObject.cpp:
+        (WebCore::RenderBoxModelObject::offsetLeft):
+        (WebCore::RenderBoxModelObject::offsetTop):
+
 2011-09-27  Tim Horton  <timothy_horton@apple.com>
 

trunk/Source/WebCore/rendering/RenderBox.cpp

@@ -1384 +1384 @@
                 o->adjustForColumns(offset, columnRect.location());
             } else
-                offset += locationOffsetIncludingFlipping();
+                offset += topLeftLocationOffset();
         } else
-            offset += locationOffsetIncludingFlipping();
+            offset += topLeftLocationOffset();
     }
 
@@ -3650 +3650 @@
 }
 
-LayoutSize RenderBox::locationOffsetIncludingFlipping() const
+LayoutPoint RenderBox::topLeftLocation() const
+{
+    RenderBlock* containerBlock = containingBlock();
+    if (!containerBlock || containerBlock == this)
+        return location();
+    return containerBlock->flipForWritingMode(this, location(), RenderBox::ParentToChildFlippingAdjustment);
+}
+
+LayoutSize RenderBox::topLeftLocationOffset() const
 {
     RenderBlock* containerBlock = containingBlock();

trunk/Source/WebCore/rendering/RenderBox.h

@@ -50 +50 @@
     LayoutUnit height() const { return m_frameRect.height(); }
 
+    // These represent your location relative to your container as a physical offset.
+    // In layout related methods you almost always want the logical location (e.g. x() and y()).
+    LayoutUnit top() const { return topLeftLocation().y(); }
+    LayoutUnit left() const { return topLeftLocation().x(); }
+
     void setX(LayoutUnit x) { m_frameRect.setX(x); }
     void setY(LayoutUnit y) { m_frameRect.setY(y); }
@@ -403 +408 @@
     FloatPoint flipForWritingMode(const FloatPoint&) const;
     void flipForWritingMode(FloatRect&) const;
-    LayoutSize locationOffsetIncludingFlipping() const;
+    // These represent your location relative to your container as a physical offset.
+    // In layout related methods you almost always want the logical location (e.g. x() and y()).
+    LayoutPoint topLeftLocation() const;
+    LayoutSize topLeftLocationOffset() const;
 
     LayoutRect logicalVisualOverflowRectForPropagation(RenderStyle*) const;

trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp

@@ -439 +439 @@
     
     RenderBoxModelObject* offsetPar = offsetParent();
-    LayoutUnit xPos = (isBox() ? toRenderBox(this)->x() : 0);
+    LayoutUnit xPos = (isBox() ? toRenderBox(this)->left() : 0);
     
     // If the offsetParent of the element is null, or is the HTML body element,
@@ -454 +454 @@
                 // FIXME: What are we supposed to do inside SVG content?
                 if (curr->isBox() && !curr->isTableRow())
-                    xPos += toRenderBox(curr)->x();
+                    xPos += toRenderBox(curr)->left();
                 curr = curr->parent();
             }
             if (offsetPar->isBox() && offsetPar->isBody() && !offsetPar->isRelPositioned() && !offsetPar->isPositioned())
-                xPos += toRenderBox(offsetPar)->x();
+                xPos += toRenderBox(offsetPar)->left();
         }
     }
@@ -473 +473 @@
     
     RenderBoxModelObject* offsetPar = offsetParent();
-    LayoutUnit yPos = (isBox() ? toRenderBox(this)->y() : 0);
+    LayoutUnit yPos = (isBox() ? toRenderBox(this)->top() : 0);
     
     // If the offsetParent of the element is null, or is the HTML body element,
@@ -488 +488 @@
                 // FIXME: What are we supposed to do inside SVG content?
                 if (curr->isBox() && !curr->isTableRow())
-                    yPos += toRenderBox(curr)->y();
+                    yPos += toRenderBox(curr)->top();
                 curr = curr->parent();
             }
             if (offsetPar->isBox() && offsetPar->isBody() && !offsetPar->isRelPositioned() && !offsetPar->isPositioned())
-                yPos += toRenderBox(offsetPar)->y();
+                yPos += toRenderBox(offsetPar)->top();
         }
     }

trunk/Source/WebCore/rendering/RenderLayer.cpp

@@ -708 +708 @@
     } else if (RenderBox* box = renderBox()) {
         setSize(box->size());
-        localPoint += box->locationOffsetIncludingFlipping();
+        localPoint += box->topLeftLocationOffset();
     }
 
@@ -722 +722 @@
                 // Rows and cells share the same coordinate space (that of the section).
                 // Omit them when computing our xpos/ypos.
-                localPoint += toRenderBox(curr)->locationOffsetIncludingFlipping();
+                localPoint += toRenderBox(curr)->topLeftLocationOffset();
             }
             curr = curr->parent();
@@ -728 +728 @@
         if (curr->isBox() && curr->isTableRow()) {
             // Put ourselves into the row coordinate space.
-            localPoint -= toRenderBox(curr)->locationOffsetIncludingFlipping();
+            localPoint -= toRenderBox(curr)->topLeftLocationOffset();
         }
     }