[Webkit-unassigned] [Bug 93170] Inline continuations create :after generated content on style recalcs

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=93170





--- Comment #21 from Abhishek Arya <inferno at chromium.org>  2012-10-02 23:42:49 PST ---
(In reply to comment #20)
> (From update of attachment 166633 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=166633&action=review
> 
> Thank you for reviewing.
> 
> >> Source/WebCore/rendering/RenderInline.cpp:177
> >> +            RenderObjectChildList::m_enableUpdateAfterContent = !nextTarget;
> > 
> > You don't need lines 176 and 177. As i said in last comment, you are explicitly updating :before, :after content later in function, so it will place it correctly at end.
> > 
> > nextTarget = currCont->inlineElementContinuation();
> > RenderObjectChildList::m_enableUpdateAfterContent = !nextTarget;
> 
> Firstly I think so. However I found that I need lines 176 and 177. Talking about the first continuation (= node()->renderer()), I agree that we don't need.
> However talking about the rest of the continuation list, this RenderInline::styleDidChange is invoked via line 180's currCont->setStyle(newStyle). 
> In such case,  later updating :before and :after content cannot see whether the RenderInline instance is the last node of continuation list. 
> 
> I think, currently there is no way to directly invoke RenderInline::styleDidChange for the second, the third continuation ...

Yes, you are right, we need to enable it for the last continuation in the chain to update :after content. Yeah upload another patch for the variable name change nit and adding comment.

> 
> >> Source/WebCore/rendering/RenderObjectChildList.cpp:379
> >> +    if (type == AFTER && !m_enableUpdateAfterContent)
> > 
> > You don't need to just check AFTER, make it generic and rename m_enableUpdateAfterContent to s_enableUpdateBeforeAfterContent. For :before content, you will bail early here instead of wasting time and going till " if (newContentWanted && type == BEFORE && owner->isElementContinuation())"
> 
> I see. 
> I added the flag to just enable/disable updating :after content, but lines 175-182 in RenderInline.cpp, we don't need to update :BEFORE. Because the continuations updated in line 175-182 cannot have :before content style.
> Done.

Exactly we know in the loop, we won't be updating :before content, so no need to waste time.

> 
> >> Source/WebCore/rendering/RenderObjectChildList.h:64
> >> +    static bool m_enableUpdateAfterContent;
> > 
> > rename to s_enableUpdateBeforeAfterContent. we s_ for statics.
> 
> Thanks.
> Done.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.