Created attachment 156496 [details] Reduction Given <span><div></div></span> with :after content on span a style recalc will cause the insertion of the content into the continuations in the div.
Created attachment 156497 [details] Counter badness
Created attachment 156499 [details] Merge continuation multiple :after This show badness where the :after content from the continuation chain is merged back into the original element since we assume the continuation isn't needed anymore, but since the content is duplication you end up with :after anonymous blocks in places where they shouldn't be (in the middle of the RenderObjectChildList). I can't figure out a way to make this crash yet, but having generated :after anonymous blocks in the middle of the children, and having more than one of them, seems like a recipe for disaster.
can you paste the rendertree before and after the style recalc. this is really interesting and i think i might have an idea about what is going wrong. I remember sometime back i discovered a bug in this area. anyway, lets discuss this in person. void RenderInline::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle) { RenderBoxModelObject::styleDidChange(diff, oldStyle); // Ensure that all of the split inlines pick up the new style. We // only do this if we're an inline, since we don't want to propagate // a block's style to the other inlines. // e.g., <font>foo <h4>goo</h4> moo</font>. The <font> inlines before // and after the block share the same style, but the block doesn't // need to pass its style on to anyone else. RenderStyle* newStyle = style(); RenderInline* continuation = inlineElementContinuation(); for (RenderInline* currCont = continuation; currCont; currCont = currCont->inlineElementContinuation()) { RenderBoxModelObject* nextCont = currCont->continuation(); currCont->setContinuation(0); currCont->setStyle(newStyle); currCont->setContinuation(nextCont); }
(In reply to comment #0) > Created an attachment (id=156496) [details] > Reduction > > Given <span><div></div></span> with :after content on span a style recalc will cause the insertion of the content into the continuations in the div. the initial rendertree should be like RenderBlock(anonymous) RenderInline (span) RenderBlock(anonymous) RenderBlock (div) RenderBlock(anonymous) RenderInline(span) RenderWhatever (for :after content) After the style recalc where does the badness happen ?
*** Bug 93707 has been marked as a duplicate of this bug. ***
Created attachment 166444 [details] Patch
(In reply to comment #4) > (In reply to comment #0) > > Created an attachment (id=156496) [details] [details] > > Reduction > > > > Given <span><div></div></span> with :after content on span a style recalc will cause the insertion of the content into the continuations in the div. > > the initial rendertree should be like > > RenderBlock(anonymous) > RenderInline (span) > RenderBlock(anonymous) > RenderBlock (div) > RenderBlock(anonymous) > RenderInline(span) > RenderWhatever (for :after content) > > After the style recalc where does the badness happen ? I ran DumpRenderTree for the following html : <style> span { display: inline; } span:before { content: "A"; } span:after { content: "B"; } </style> <span> <div></div> <div></div> </span> Since ":before" and ":after" exist, the result looks like: RenderBlock {HTML} at (0,0) size 800x600 RenderBody {BODY} at (8,8) size 784x584 RenderBlock (anonymous) at (0,0) size 784x20 RenderInline {SPAN} at (0,0) size 11x19 RenderInline (generated) at (0,0) size 11x19 RenderText at (0,0) size 11x19 text run at (0,0) width 11: "A" RenderText {#text} at (0,0) size 0x0 RenderBlock (anonymous) at (0,20) size 784x0 RenderBlock {DIV} at (0,0) size 784x0 RenderBlock (anonymous) at (0,20) size 784x0 RenderInline {SPAN} at (0,0) size 0x0 RenderBlock (anonymous) at (0,20) size 784x0 RenderBlock {DIV} at (0,0) size 784x0 RenderBlock (anonymous) at (0,20) size 784x20 RenderInline {SPAN} at (0,0) size 11x19 RenderInline (generated) at (0,0) size 11x19 RenderText at (0,0) size 11x19 text run at (0,0) width 11: "B" So there are 3 RenderInline{SPAN}, because two DIVs were inserted into inline {SPAN}. (Each DIV splits the last RenderInline{SPAN} into two RenderInline{SPAN}s by using RenderBlock (anonymous). If <style> has neither :BEFORE nor :AFTER, the "split" doesn't occur.) When RenderInline::styleDidChange is invoked, because of setContinuation(0), there is no way to know whether the given continuation is last one or not. So... RenderBlock {HTML} at (0,0) size 800x600 RenderBody {BODY} at (8,8) size 784x584 RenderBlock (anonymous) at (0,0) size 784x20 RenderInline {SPAN} at (0,0) size 11x19 RenderInline (generated) at (0,0) size 11x19 RenderText at (0,0) size 11x19 text run at (0,0) width 11: "A" RenderText {#text} at (0,0) size 0x0 RenderBlock (anonymous) at (0,20) size 784x0 RenderBlock {DIV} at (0,0) size 784x0 RenderBlock (anonymous) at (0,20) size 784x0 RenderInline {SPAN} at (0,0) size 0x0 + RenderInline (generated) at (0,0) size 11x19 + RenderText at (0,0) size 11x19 + text run at (0,0) width 11: "B" RenderBlock (anonymous) at (0,20) size 784x0 RenderBlock {DIV} at (0,0) size 784x0 RenderBlock (anonymous) at (0,20) size 784x20 RenderInline {SPAN} at (0,0) size 11x19 RenderInline (generated) at (0,0) size 11x19 RenderText at (0,0) size 11x19 text run at (0,0) width 11: "B" Best regards, Takashi Sakamoto
Comment on attachment 166444 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=166444&action=review > Source/WebCore/rendering/RenderInline.cpp:176 > + if (hasPseudoAfter && !currCont->inlineElementContinuation()) These two lines are unneeded i think since you update :before, :after content later. 197 if (!isAnonymous() && document()->styleSheetCollection()->usesBeforeAfterRules()) { 198 children()->updateBeforeAfterContent(this, BEFORE); 199 children()->updateBeforeAfterContent(this, AFTER); 200 } > Source/WebCore/rendering/RenderInline.cpp:182 > + if (hasPseudoAfter) I don't like the setting/unsetting :after style hack. Also, i don't think we want to create :before content here. So, it should be better to have like a global bool in RenderObjectChildList and then use it to bail out in RenderObjectChildList::updateBeforeAfterContent. Then you can set/unset it here with a TemporaryChange around the for loop. Basically the gist is we shouldn't be updating our :before, :after while we are inside the for loop.
Created attachment 166603 [details] Patch
Comment on attachment 166444 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=166444&action=review Thank you for reviewing. >> Source/WebCore/rendering/RenderInline.cpp:176 >> + if (hasPseudoAfter && !currCont->inlineElementContinuation()) > > These two lines are unneeded i think since you update :before, :after content later. > > 197 if (!isAnonymous() && document()->styleSheetCollection()->usesBeforeAfterRules()) { > 198 children()->updateBeforeAfterContent(this, BEFORE); > 199 children()->updateBeforeAfterContent(this, AFTER); > 200 } I see. This is my mistake. >> Source/WebCore/rendering/RenderInline.cpp:182 >> + if (hasPseudoAfter) > > I don't like the setting/unsetting :after style hack. Also, i don't think we want to create :before content here. So, it should be better to have like a global bool in RenderObjectChildList and then use it to bail out in RenderObjectChildList::updateBeforeAfterContent. Then you can set/unset it here with a TemporaryChange around the for loop. Basically the gist is we shouldn't be updating our :before, :after while we are inside the for loop. I see. I added a new static method, i.e. enableUpdateBeforeAfterContent to class RenderObjectChildList to control creating :before / :after content.
Comment on attachment 166603 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=166603&action=review One more round. > Source/WebCore/rendering/RenderInline.cpp:169 > + bool updateEnabled = RenderObjectChildList::enableUpdateBeforeAfterContent(false); you can use TemporaryChange to scope this easily. see comment below. > Source/WebCore/rendering/RenderObjectChildList.cpp:443 > + if (!m_enableUpdateBeforeAfterContent) Why so late return ? Why not after the line " if (owner->style()->styleType() == BEFORE || owner->style()->styleType() == AFTER)" > Source/WebCore/rendering/RenderObjectChildList.h:59 > + static bool enableUpdateBeforeAfterContent(bool enable) You don't need this. Just use TemporaryChange.h, see http://code.google.com/codesearch#OAMlx_jo-ck/src/third_party/WebKit/Tools/TestWebKitAPI/Tests/WTF/TemporaryChange.cpp&exact_package=chromium&q=%22TemporaryChange%3Cbool%22&type=cs&l=36. Make m_enableUpdateBeforeAfterContent a global static that could be manipulated by RenderInline::styleDidChange.
Created attachment 166612 [details] Patch
Created attachment 166633 [details] Patch
I'm sorry. I said wrong things. I looked at my old patch for bug 93707 again and found that we need to enable updating :after content during curr->setStyle. To show why we need, I added one more layout test to this patch. The test updates className to apply / not to apply :after to inline elements. If we disable updating :after content in curr->setStyle, no generated renderer for :after content is created. Best regards, Takashi Sakamoto (In reply to comment #11) > (From update of attachment 166603 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=166603&action=review > > One more round. > > > Source/WebCore/rendering/RenderInline.cpp:169 > > + bool updateEnabled = RenderObjectChildList::enableUpdateBeforeAfterContent(false); > > you can use TemporaryChange to scope this easily. see comment below. > > > Source/WebCore/rendering/RenderObjectChildList.cpp:443 > > + if (!m_enableUpdateBeforeAfterContent) > > Why so late return ? Why not after the line " if (owner->style()->styleType() == BEFORE || owner->style()->styleType() == AFTER)" > > > Source/WebCore/rendering/RenderObjectChildList.h:59 > > + static bool enableUpdateBeforeAfterContent(bool enable) > > You don't need this. Just use TemporaryChange.h, see http://code.google.com/codesearch#OAMlx_jo-ck/src/third_party/WebKit/Tools/TestWebKitAPI/Tests/WTF/TemporaryChange.cpp&exact_package=chromium&q=%22TemporaryChange%3Cbool%22&type=cs&l=36. Make m_enableUpdateBeforeAfterContent a global static that could be manipulated by RenderInline::styleDidChange.
I forgot to say about :before content. The reason why we don't need to do anything for :before content in RenderInline::styleDidChange is that it is easy to know whether the given render object is the top renderer of continuation list. Just compare renderer with renderer->node()->renderer(). This is not affected by setContinuation(0). Best regards, Takashi Sakamoto (In reply to comment #14) > I'm sorry. I said wrong things. > I looked at my old patch for bug 93707 again and found that we need to enable updating :after content during curr->setStyle. > To show why we need, I added one more layout test to this patch. The test updates className to apply / not to apply :after to inline elements. If we disable updating :after content in curr->setStyle, no generated renderer for :after content is created. > > Best regards, > Takashi Sakamoto > > (In reply to comment #11) > > (From update of attachment 166603 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=166603&action=review > > > > One more round. > > > > > Source/WebCore/rendering/RenderInline.cpp:169 > > > + bool updateEnabled = RenderObjectChildList::enableUpdateBeforeAfterContent(false); > > > > you can use TemporaryChange to scope this easily. see comment below. > > > > > Source/WebCore/rendering/RenderObjectChildList.cpp:443 > > > + if (!m_enableUpdateBeforeAfterContent) > > > > Why so late return ? Why not after the line " if (owner->style()->styleType() == BEFORE || owner->style()->styleType() == AFTER)" > > > > > Source/WebCore/rendering/RenderObjectChildList.h:59 > > > + static bool enableUpdateBeforeAfterContent(bool enable) > > > > You don't need this. Just use TemporaryChange.h, see http://code.google.com/codesearch#OAMlx_jo-ck/src/third_party/WebKit/Tools/TestWebKitAPI/Tests/WTF/TemporaryChange.cpp&exact_package=chromium&q=%22TemporaryChange%3Cbool%22&type=cs&l=36. Make m_enableUpdateBeforeAfterContent a global static that could be manipulated by RenderInline::styleDidChange.
I wrote the document to explain this patch: https://docs.google.com/document/d/1YsXo7dAs7EOdckXy27Z8b9KlydT1p5SZH0fARudkFoI/edit (In reply to comment #15) > I forgot to say about :before content. > > The reason why we don't need to do anything for :before content in RenderInline::styleDidChange is that it is easy to know whether the given render object is the top renderer of continuation list. > Just compare renderer with renderer->node()->renderer(). This is not affected by setContinuation(0). > > Best regards, > Takashi Sakamoto > > (In reply to comment #14) > > I'm sorry. I said wrong things. > > I looked at my old patch for bug 93707 again and found that we need to enable updating :after content during curr->setStyle. > > To show why we need, I added one more layout test to this patch. The test updates className to apply / not to apply :after to inline elements. If we disable updating :after content in curr->setStyle, no generated renderer for :after content is created. > > > > Best regards, > > Takashi Sakamoto > > > > (In reply to comment #11) > > > (From update of attachment 166603 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=166603&action=review > > > > > > One more round. > > > > > > > Source/WebCore/rendering/RenderInline.cpp:169 > > > > + bool updateEnabled = RenderObjectChildList::enableUpdateBeforeAfterContent(false); > > > > > > you can use TemporaryChange to scope this easily. see comment below. > > > > > > > Source/WebCore/rendering/RenderObjectChildList.cpp:443 > > > > + if (!m_enableUpdateBeforeAfterContent) > > > > > > Why so late return ? Why not after the line " if (owner->style()->styleType() == BEFORE || owner->style()->styleType() == AFTER)" > > > > > > > Source/WebCore/rendering/RenderObjectChildList.h:59 > > > > + static bool enableUpdateBeforeAfterContent(bool enable) > > > > > > You don't need this. Just use TemporaryChange.h, see http://code.google.com/codesearch#OAMlx_jo-ck/src/third_party/WebKit/Tools/TestWebKitAPI/Tests/WTF/TemporaryChange.cpp&exact_package=chromium&q=%22TemporaryChange%3Cbool%22&type=cs&l=36. Make m_enableUpdateBeforeAfterContent a global static that could be manipulated by RenderInline::styleDidChange.
Comment on attachment 166633 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=166633&action=review > 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; > 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())" > Source/WebCore/rendering/RenderObjectChildList.h:64 > + static bool m_enableUpdateAfterContent; rename to s_enableUpdateBeforeAfterContent. we s_ for statics.
(In reply to comment #15) > I forgot to say about :before content. > > The reason why we don't need to do anything for :before content in RenderInline::styleDidChange is that it is easy to know whether the given render object is the top renderer of continuation list. > Just compare renderer with renderer->node()->renderer(). This is not affected by setContinuation(0). Yeah i know it will bail out on "if (newContentWanted && type == BEFORE && owner->isElementContinuation())", but we will waste on every continuation calculating before content, when we could bail out early. switch (type) { case BEFORE: child = beforePseudoElementRenderer(owner); break; > > Best regards, > Takashi Sakamoto > > (In reply to comment #14) > > I'm sorry. I said wrong things. > > I looked at my old patch for bug 93707 again and found that we need to enable updating :after content during curr->setStyle. > > To show why we need, I added one more layout test to this patch. The test updates className to apply / not to apply :after to inline elements. If we disable updating :after content in curr->setStyle, no generated renderer for :after content is created. > > > > Best regards, > > Takashi Sakamoto > > > > (In reply to comment #11) > > > (From update of attachment 166603 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=166603&action=review > > > > > > One more round. > > > > > > > Source/WebCore/rendering/RenderInline.cpp:169 > > > > + bool updateEnabled = RenderObjectChildList::enableUpdateBeforeAfterContent(false); > > > > > > you can use TemporaryChange to scope this easily. see comment below. > > > > > > > Source/WebCore/rendering/RenderObjectChildList.cpp:443 > > > > + if (!m_enableUpdateBeforeAfterContent) > > > > > > Why so late return ? Why not after the line " if (owner->style()->styleType() == BEFORE || owner->style()->styleType() == AFTER)" > > > > > > > Source/WebCore/rendering/RenderObjectChildList.h:59 > > > > + static bool enableUpdateBeforeAfterContent(bool enable) > > > > > > You don't need this. Just use TemporaryChange.h, see http://code.google.com/codesearch#OAMlx_jo-ck/src/third_party/WebKit/Tools/TestWebKitAPI/Tests/WTF/TemporaryChange.cpp&exact_package=chromium&q=%22TemporaryChange%3Cbool%22&type=cs&l=36. Make m_enableUpdateBeforeAfterContent a global static that could be manipulated by RenderInline::styleDidChange.
Created attachment 166809 [details] Patch
Comment on attachment 166633 [details] Patch 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 ... >> 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. >> Source/WebCore/rendering/RenderObjectChildList.h:64 >> + static bool m_enableUpdateAfterContent; > > rename to s_enableUpdateBeforeAfterContent. we s_ for statics. Thanks. Done.
(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.
Created attachment 166815 [details] Patch
Comment on attachment 166809 [details] Patch 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? > 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. > LayoutTests/fast/css-generated-content/after-with-inline-continuation.html:45 > +<!-- If test passes, you should see A A A B B B. --> 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.
(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.
Comment on attachment 166815 [details] Patch Don't think we should hold on the patch based on the layout test suggestions. The other question i did seem to answer. So, this one going to cq+
Comment on attachment 166815 [details] Patch Clearing flags on attachment: 166815 Committed r130555: <http://trac.webkit.org/changeset/130555>
All reviewed patches have been landed. Closing bug.