REGRESSION: RenderInline boundingBox ignores relative position offset
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 14 Jul 2012 01:08:07 +0000 (01:08 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 14 Jul 2012 01:08:07 +0000 (01:08 +0000)
https://bugs.webkit.org/show_bug.cgi?id=91168

Patch by Kiran Muppala <cmuppala@apple.com> on 2012-07-13
Reviewed by Simon Fraser.

Source/WebCore:

RenderGeometryMap, used for caching the transform to the view,
expects the first mapping pushed, to be that of the view itself.
RenderInline was instead pushing it's own offset first.  Besides
the offset of the view itself was not being pushed.

Relaxed the RenderGeometryMap restriction that the first pushed
step should be of the view.  It is sufficient that the view's mapping
is pushed in the first call to pushMappingsToAncestor.  Modified
RenderInline to push the offset of the view also to the geometry map.

Test: fast/inline/inline-relative-offset-boundingbox.html

* rendering/RenderGeometryMap.cpp:
(WebCore::RenderGeometryMap::pushMappingsToAncestor): Add assertion to
check if mapping to view was pushed in first invocation.
(WebCore::RenderGeometryMap::pushView): Correct assertion that checks
if the view's mapping is the first one to be applied.
(WebCore::RenderGeometryMap::stepInserted): Use isRenderView to check if
a mapping step belongs to a view instead of using mapping size.
(WebCore::RenderGeometryMap::stepRemoved): Ditto.
* rendering/RenderInline.cpp:
(WebCore::(anonymous namespace)::AbsoluteQuadsGeneratorContext::AbsoluteQuadsGeneratorContext):
Push mappings all the way up to and including the view.

LayoutTests:

Add a regression test for boundingBox of an inline element with relative position offsets.

* fast/inline/inline-relative-offset-boundingbox-expected.txt: Added.
* fast/inline/inline-relative-offset-boundingbox.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@122653 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/inline/inline-relative-offset-boundingbox-expected.txt [new file with mode: 0644]
LayoutTests/fast/inline/inline-relative-offset-boundingbox.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderGeometryMap.cpp
Source/WebCore/rendering/RenderInline.cpp

index ecfebc8..0f1e7aa 100644 (file)
@@ -1,3 +1,15 @@
+2012-07-13  Kiran Muppala  <cmuppala@apple.com>
+
+        REGRESSION: RenderInline boundingBox ignores relative position offset
+        https://bugs.webkit.org/show_bug.cgi?id=91168
+
+        Reviewed by Simon Fraser.
+
+        Add a regression test for boundingBox of an inline element with relative position offsets.
+
+        * fast/inline/inline-relative-offset-boundingbox-expected.txt: Added.
+        * fast/inline/inline-relative-offset-boundingbox.html: Added.
+
 2012-07-13  Xianzhu Wang  <wangxianzhu@chromium.org>
 
         [Chromium] Sometimes bottom of text is truncated when page has a fractional scale
diff --git a/LayoutTests/fast/inline/inline-relative-offset-boundingbox-expected.txt b/LayoutTests/fast/inline/inline-relative-offset-boundingbox-expected.txt
new file mode 100644 (file)
index 0000000..21a2fa4
--- /dev/null
@@ -0,0 +1,10 @@
+Bug 91168: REGRESSION: RenderInline boundingBox ignores relative position offset
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+PASS inlineRect.left is parentRect.left + inlineLeftOffset
+
diff --git a/LayoutTests/fast/inline/inline-relative-offset-boundingbox.html b/LayoutTests/fast/inline/inline-relative-offset-boundingbox.html
new file mode 100644 (file)
index 0000000..d6933c6
--- /dev/null
@@ -0,0 +1,24 @@
+<html>
+<head>
+    <script src="../js/resources/js-test-pre.js"></script>
+    <script>
+        description('<a href="https://bugs.webkit.org/show_bug.cgi?id=91168">Bug 91168</a>: REGRESSION: RenderInline boundingBox ignores relative position offset');
+
+        function runTest()
+        {
+            inline = document.getElementById("inlineElement");
+            inlineRect = inline.getBoundingClientRect();
+            inlineLeftOffset = parseInt(inline.style.left);
+            parent = inline.parentNode;
+            parentRect = parent.getBoundingClientRect();
+            shouldBe("inlineRect.left", "parentRect.left + inlineLeftOffset");
+        }
+
+        window.onload = runTest;
+    </script>
+</head>
+<body>
+    <span id="inlineElement" style="position:relative; left:10px;"></span>
+    <script src="../js/resources/js-test-post.js"></script>
+</body>
+</html>
index 758b1f5..591bc02 100644 (file)
@@ -1,3 +1,34 @@
+2012-07-13  Kiran Muppala  <cmuppala@apple.com>
+
+        REGRESSION: RenderInline boundingBox ignores relative position offset
+        https://bugs.webkit.org/show_bug.cgi?id=91168
+
+        Reviewed by Simon Fraser.
+
+        RenderGeometryMap, used for caching the transform to the view,
+        expects the first mapping pushed, to be that of the view itself.
+        RenderInline was instead pushing it's own offset first.  Besides
+        the offset of the view itself was not being pushed.
+
+        Relaxed the RenderGeometryMap restriction that the first pushed
+        step should be of the view.  It is sufficient that the view's mapping
+        is pushed in the first call to pushMappingsToAncestor.  Modified
+        RenderInline to push the offset of the view also to the geometry map.
+
+        Test: fast/inline/inline-relative-offset-boundingbox.html
+
+        * rendering/RenderGeometryMap.cpp:
+        (WebCore::RenderGeometryMap::pushMappingsToAncestor): Add assertion to
+        check if mapping to view was pushed in first invocation.
+        (WebCore::RenderGeometryMap::pushView): Correct assertion that checks
+        if the view's mapping is the first one to be applied.
+        (WebCore::RenderGeometryMap::stepInserted): Use isRenderView to check if
+        a mapping step belongs to a view instead of using mapping size.
+        (WebCore::RenderGeometryMap::stepRemoved): Ditto.
+        * rendering/RenderInline.cpp:
+        (WebCore::(anonymous namespace)::AbsoluteQuadsGeneratorContext::AbsoluteQuadsGeneratorContext):
+        Push mappings all the way up to and including the view.
+
 2012-07-13  Xianzhu Wang  <wangxianzhu@chromium.org>
 
         Move WebCore/platform/text/Base64 to WTF/wtf/text
index 8c6fa0b..0a6c9e0 100644 (file)
@@ -136,6 +136,8 @@ void RenderGeometryMap::pushMappingsToAncestor(const RenderObject* renderer, con
     do {
         renderer = renderer->pushMappingToContainer(ancestorRenderer, *this);
     } while (renderer && renderer != ancestorRenderer);
+
+    ASSERT(m_mapping.isEmpty() || m_mapping[0].m_renderer->isRenderView());
 }
 
 static bool canMapViaLayer(const RenderLayer* layer)
@@ -205,7 +207,7 @@ void RenderGeometryMap::push(const RenderObject* renderer, const TransformationM
 void RenderGeometryMap::pushView(const RenderView* view, const LayoutSize& scrollOffset, const TransformationMatrix* t)
 {
     ASSERT(m_insertionPosition != notFound);
-    ASSERT(!m_mapping.size()); // The view should always be the first thing pushed.
+    ASSERT(!m_insertionPosition); // The view should always be the first step.
 
     m_mapping.insert(m_insertionPosition, RenderGeometryMapStep(view, false, false, false, t));
     
@@ -235,8 +237,8 @@ void RenderGeometryMap::popMappingsToAncestor(const RenderLayer* ancestorLayer)
 
 void RenderGeometryMap::stepInserted(const RenderGeometryMapStep& step)
 {
-    // Offset on the first step is the RenderView's offset, which is only applied when we have fixed-position.s
-    if (m_mapping.size() > 1)
+    // RenderView's offset, is only applied when we have fixed-positions.
+    if (!step.m_renderer->isRenderView())
         m_accumulatedOffset += step.m_offset;
 
     if (step.m_isNonUniform)
@@ -251,8 +253,8 @@ void RenderGeometryMap::stepInserted(const RenderGeometryMapStep& step)
 
 void RenderGeometryMap::stepRemoved(const RenderGeometryMapStep& step)
 {
-    // Offset on the first step is the RenderView's offset, which is only applied when we have fixed-position.s
-    if (m_mapping.size() > 1)
+    // RenderView's offset, is only applied when we have fixed-positions.
+    if (!step.m_renderer->isRenderView())
         m_accumulatedOffset -= step.m_offset;
 
     if (step.m_isNonUniform) {
index 3032480..c4e1423 100644 (file)
@@ -652,12 +652,7 @@ public:
         , m_wasFixed(wasFixed)
         , m_geometryMap()
     {
-        RenderObject* root = renderer->parent();
-        while (root && root->parent())
-            root = root->parent();
-
-        if (root)
-            m_geometryMap.pushMappingsToAncestor(renderer, toRenderBoxModelObject(root));
+        m_geometryMap.pushMappingsToAncestor(renderer, 0);
     }
 
     void operator()(const FloatRect& rect)