Bug 91168

Summary: REGRESSION: RenderInline boundingBox ignores relative position offset
Product: WebKit Reporter: Kiran Muppala <cmuppala>
Component: Layout and RenderingAssignee: Kiran Muppala <cmuppala>
Status: RESOLVED FIXED    
Severity: Normal CC: cmuppala, eric, simon.fraser, webkit-bug-importer, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Kiran Muppala 2012-07-12 17:11:56 PDT
RenderInline::absoluteQuads was optimized to use a RenderGeometryMap for caching the transform for local to absolute coordinate conversion by https://bugs.webkit.org/show_bug.cgi?id=90715.  But since that change, the bounding box returned is not adding the relative position offset of the inline
element itself, if any.  This leads to assertion failures when opening several webpages, including https://developer.mozilla.org/en/DOM/element.getBoundingClientRect.

The root cause for this issue is that the GeometryMap expects the first mapping pushed to it be that of the RenderView, whereas RenderInline::absoluteQuads
is pushing it's own mapping first, leading to the observed discrepancy.
Comment 1 Kiran Muppala 2012-07-12 17:13:25 PDT
<rdar://problem/11845860>
Comment 2 Kiran Muppala 2012-07-12 18:44:37 PDT
Created attachment 152127 [details]
Patch
Comment 3 Simon Fraser (smfr) 2012-07-13 09:06:53 PDT
Comment on attachment 152127 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=152127&action=review

> Source/WebCore/rendering/RenderInline.cpp:660
> +            m_geometryMap.pushMappingsToAncestor(view, 0);
> +            m_geometryMap.pushMappingsToAncestor(renderer, view);
> +        }

Does m_geometryMap.pushMappingsToAncestor(renderer, 0); not work? If so, we should just fix RenderGeometryMap so that it does.
Comment 4 Kiran Muppala 2012-07-13 12:15:40 PDT
(In reply to comment #3)
> (From update of attachment 152127 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=152127&action=review
> 
> > Source/WebCore/rendering/RenderInline.cpp:660
> > +            m_geometryMap.pushMappingsToAncestor(view, 0);
> > +            m_geometryMap.pushMappingsToAncestor(renderer, view);
> > +        }
> 
> Does m_geometryMap.pushMappingsToAncestor(renderer, 0); not work? If so, we should just fix RenderGeometryMap so that it does.

Yes, calling m_geometryMap.pushMappingsToAncestor(renderer, 0) results in the renderer's mapping pushed first, which is exactly what is happening currently.  In fact the assertion, ASSERT(!m_mapping.size()), in RenderGeometryMap::pushView fails for such a call.
Comment 5 Kiran Muppala 2012-07-13 15:17:03 PDT
Created attachment 152351 [details]
Patch
Comment 6 Simon Fraser (smfr) 2012-07-13 16:05:38 PDT
Comment on attachment 152351 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=152351&action=review

> LayoutTests/fast/inline/inline-relative-offset-boundingbox.html:14
> +            parentRect = parent.getBoundingClientRect();
> +	    shouldBe("inlineRect.left", "parentRect.left + inlineLeftOffset");

Indentation looks wrong here.
Comment 7 Kiran Muppala 2012-07-13 16:10:09 PDT
Created attachment 152364 [details]
Patch
Comment 8 Kiran Muppala 2012-07-13 16:12:39 PDT
(In reply to comment #6)
> (From update of attachment 152351 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=152351&action=review
> 
> > LayoutTests/fast/inline/inline-relative-offset-boundingbox.html:14
> > +            parentRect = parent.getBoundingClientRect();
> > +	    shouldBe("inlineRect.left", "parentRect.left + inlineLeftOffset");
> 
> Indentation looks wrong here.

My bad, there was a tab in the second line.  I replaced it with spaces and that fixed the indentation.

Uploaded a new patch.  Only change is the fix for indentation.
Comment 9 WebKit Review Bot 2012-07-13 18:08:18 PDT
Comment on attachment 152364 [details]
Patch

Clearing flags on attachment: 152364

Committed r122653: <http://trac.webkit.org/changeset/122653>
Comment 10 WebKit Review Bot 2012-07-13 18:08:23 PDT
All reviewed patches have been landed.  Closing bug.