Changeset 122653 in webkit

Timestamp:

Jul 13, 2012 6:08:07 PM (12 years ago)

Author:

commit-queue@webkit.org

Message:

REGRESSION: RenderInline boundingBox ignores relative position offset
​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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/inline/inline-relative-offset-boundingbox-expected.txt (added)
• LayoutTests/fast/inline/inline-relative-offset-boundingbox.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderGeometryMap.cpp (modified) (4 diffs)
• Source/WebCore/rendering/RenderInline.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/rendering/RenderGeometryMap.cpp

@@ -137 +137 @@
         renderer = renderer->pushMappingToContainer(ancestorRenderer, *this);
     } while (renderer && renderer != ancestorRenderer);
+
+    ASSERT(m_mapping.isEmpty() || m_mapping[0].m_renderer->isRenderView());
 }
 
@@ -206 +208 @@
 {
     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));
@@ -236 +238 @@
 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;
 
@@ -252 +254 @@
 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;
 

trunk/Source/WebCore/rendering/RenderInline.cpp

@@ -653 +653 @@
         , 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);
     }