This is a hack to track state change between styleWillChange and styleDidChange.
<rdar://problem/37961079>
Created attachment 334712 [details] Patch
Created attachment 334724 [details] Patch
Comment on attachment 334724 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334724&action=review > Source/WebCore/rendering/RenderElement.cpp:-97 > -bool RenderElement::s_affectsParentBlock = false; Any reason why you do the removal of s_noLongerAffectsParentBlock in the separate bug 183196? Or it's just to be careful and do smaller changes? > Source/WebCore/rendering/RenderElement.cpp:-773 > - s_affectsParentBlock = false; Normally you should remove braces per https://webkit.org/code-style-guidelines/#braces-one-line although I understand this else will eventually go away. > Source/WebCore/rendering/RenderElement.cpp:812 > + && parent() && (parent()->isRenderBlockFlow() || parent()->isRenderInline())) { I wonder whether keeping a local affectsParentBlock variable would help making the meaning of this big conditional more explicit or whether the comment is enough.
r=me
(In reply to Frédéric Wang (:fredw) from comment #4) > Comment on attachment 334724 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=334724&action=review > > > Source/WebCore/rendering/RenderElement.cpp:-97 > > -bool RenderElement::s_affectsParentBlock = false; > > Any reason why you do the removal of s_noLongerAffectsParentBlock in the > separate bug 183196? Or it's just to be careful and do smaller changes? > just being careful (moving the s_noLongerAffectsParentBlock needs a bit more work since it has a bit of a dependency) > > Source/WebCore/rendering/RenderElement.cpp:-773 > > - s_affectsParentBlock = false; > > Normally you should remove braces per > https://webkit.org/code-style-guidelines/#braces-one-line although I > understand this else will eventually go away. Yeah, I'll remove the braces anyway. > > > Source/WebCore/rendering/RenderElement.cpp:812 > > + && parent() && (parent()->isRenderBlockFlow() || parent()->isRenderInline())) { > > I wonder whether keeping a local affectsParentBlock variable would help > making the meaning of this big conditional more explicit or whether the > comment is enough. I was thinking about that too but for some reason I decided not to. but now I am leaning towards having the variable there.
(In reply to zalan from comment #6) > (In reply to Frédéric Wang (:fredw) from comment #4) > > Comment on attachment 334724 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=334724&action=review > > > > > Source/WebCore/rendering/RenderElement.cpp:-97 > > > -bool RenderElement::s_affectsParentBlock = false; > > > > Any reason why you do the removal of s_noLongerAffectsParentBlock in the > > separate bug 183196? Or it's just to be careful and do smaller changes? > > > just being careful (moving the s_noLongerAffectsParentBlock needs a bit more > work since it has a bit of a dependency) > > > > Source/WebCore/rendering/RenderElement.cpp:-773 > > > - s_affectsParentBlock = false; > > > > Normally you should remove braces per > > https://webkit.org/code-style-guidelines/#braces-one-line although I > > understand this else will eventually go away. > Yeah, I'll remove the braces anyway. > > > > > > Source/WebCore/rendering/RenderElement.cpp:812 > > > + && parent() && (parent()->isRenderBlockFlow() || parent()->isRenderInline())) { > > > > I wonder whether keeping a local affectsParentBlock variable would help > > making the meaning of this big conditional more explicit or whether the > > comment is enough. > I was thinking about that too but for some reason I decided not to. but now > I am leaning towards having the variable there. Though this is all temporary anyway since the point of this exercise is to move the mutation logic to the tree updater.
Created attachment 334744 [details] Patch
Comment on attachment 334744 [details] Patch Clearing flags on attachment: 334744 Committed r229091: <https://trac.webkit.org/changeset/229091>
All reviewed patches have been landed. Closing bug.