m_buttonText and m_inner
<rdar://problem/34840807>
Created attachment 322888 [details] Patch
Comment on attachment 322888 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=322888&action=review > Source/WebCore/rendering/RenderButton.cpp:83 > // violated. > if (&oldChild == m_inner || !m_inner || oldChild.parent() == this) { > ASSERT(&oldChild == m_inner || !m_inner); The comment above talks about "security problems" that I don't think exist with the recent improvements. Maybe the comment, oldChild.parent() == this test and the assert can go?
Comment on attachment 322888 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=322888&action=review > Source/WebCore/rendering/RenderButton.cpp:139 > + return { }; Is this as efficient as returning emptyString()? > Source/WebCore/rendering/RenderButton.h:74 > + WeakPtr<RenderBlock> m_inner; Do we actually need to reference this, or can we find it via a tree walk?
Comment on attachment 322888 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=322888&action=review >> Source/WebCore/rendering/RenderButton.cpp:139 >> + return { }; > > Is this as efficient as returning emptyString()? That returns a null string, not an empty string (and is more efficient).
(In reply to Antti Koivisto from comment #3) > Comment on attachment 322888 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=322888&action=review > > > Source/WebCore/rendering/RenderButton.cpp:83 > > // violated. > > if (&oldChild == m_inner || !m_inner || oldChild.parent() == this) { > > ASSERT(&oldChild == m_inner || !m_inner); > > The comment above talks about "security problems" that I don't think exist > with the recent improvements. Maybe the comment, oldChild.parent() == this > test and the assert can go? The comment/assert actually attempts to catch cases where the RenderButton's inner subtree gets split up for whatever reason (the original bug was column spanning). So I think this is still valid (though not super useful, I admit).
Comment on attachment 322888 [details] Patch Clearing flags on attachment: 322888 Committed r222932: <http://trac.webkit.org/changeset/222932>
All reviewed patches have been landed. Closing bug.