[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 #24 from Abhishek Arya <inferno at chromium.org>  2012-10-03 08:03:41 PST ---
(In reply to comment #23)
> (From update of attachment 166809 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=166809&action=review
> 
> > Source/WebCore/rendering/RenderInline.cpp:177
> > +            RenderObjectChildList::s_enableUpdateBeforeAfterContent = !nextTarget;
> 
> This naming isn't clear. It's not a target of anything and that naming is confusing in the DOM/Rendering area. Could we use next and current instead?

He fixed that in last patch. i had told him informally over chat.

> 
> > Source/WebCore/rendering/RenderInline.cpp:181
> > +            currCont->setContinuation(nextCont);
> 
> Is it ever possible that nextCont is not null, but nextTarget is (meaning nextCont is not isInline())? Looking at the code it doesn't seem like you could have a continuation chain that ends in a non-inline one here which means you could simplify this loop a bunch.
> 
> Try adding ASSERT(!current->continuation() || current->inlineElementContinuation()); to this loop and running the tests.

We cannot have non-inline as the last continuation. These continuations are created for the blocks inside inline case [Blocks inside Inline Flows in https://www.webkit.org/blog/115/webcore-rendering-ii-blocks-and-inlines/]. Also, i did thought about simplifying the loop but it does not look straightforward, we need to jump to every next inline element continuation [so it will skip over the block inside inline and also update styles of the inline continuation], whereas the nextCont is always the next continuation which can be the block inside inline or the inline continuation. What was your idea on the simplification ?

> 
> > LayoutTests/fast/css-generated-content/after-with-inline-continuation.html:45
> > +<!-- If test passes, you should see A A A B B B. -->
> 

Thanks for the layout test suggestions.

> Same, this should be inline like the original test.
> 
> > LayoutTests/fast/css-generated-content/dynamic-apply-after-for-inline.html:23
> > +        ul.className = '';
> 
> Replace all this with ul.classList.toggle('closed')
> 
> > LayoutTests/fast/css-generated-content/dynamic-apply-after-for-inline.html:45
> > +<!-- If test passes, only 1 'before' and 1 'after' are shown. -->
> 
> Can you put this text inline instead of in a comment?
> 
> <p>....</p> It's much easier to tell what's supposed to be going on.

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