The content property in this example causes WebKit to hang. The example uses this: #submit:before { content: "\2714"; } #clear:before { content: "\2718"; text-shadow: black 0.2em 0.1em 0.1em } And on load the app hangs. Removing the content property allows the page to load normally. This works as expected in Safari 2.0.3 and Camino.
Created attachment 8993 [details] Example document.
Created attachment 8998 [details] Reduction of test case, non-aqua button Hovering the button in this test case will hang Safari.
Created attachment 8999 [details] Reduction of test case, aqua button When reducing the test case even further, the button reverts to an aqua look, and doesn't seem to react to hovering at all, so WebKit doesn't hang. Clicking the button hangs WebKit, though.
I just reduced the original example a bit, to make it more to the point. It seems that it is the style change on hovering or clicking that triggers the hang.
(In reply to comment #3) > Created an attachment (id=8999) [edit] > Reduction of test case, aqua button A debug build fails the assertion KHTMLAssert(beforeChild->parent()->isAnonymousBlock()) in RenderBlock::addChildToFlow() when I click the button.
Created attachment 9019 [details] Possible fix, no test or change log yet This patch is somewhat cumbersome. Another approach I considered was to patch RenderButton::setStyle() and RenderButton::addChild() to copy the pseudostyles to the inner div, but I had a feeling it would end up being at least as complicated as this patch's approach.
Comment on attachment 9019 [details] Possible fix, no test or change log yet I'm not a big fan of functions that have those "do" prefixes. It's always hard to know how they differ from the version without the "do" prefix. It's just "stage 2". We should seek out a better name for the non-virtual core implementation of updatePseudoChild. In what cases will it ever be good for RenderButton to call doUpdatePseudoChild on itself rather than on m_inner? Architecturally, the worst thing here is the knowledge in RenderContainer::updatePseudoChild that the parent is special when it's a RenderButton. That needs a comment at least. And we should see what we can do to avoid this new slightly-fragile linkage between the classes. Even having a virtual function that was named something cumbersome like "updatesPseudoChildForChildren" might be better.
(In reply to comment #7) > We should seek out a better name for the non-virtual core > implementation of updatePseudoChild. I'll give it more thought. > In what cases will it ever be good for RenderButton to call doUpdatePseudoChild > on itself rather than on m_inner? <style> button:hover:before { content: 'foo' } </style> <button></button> The first time you hover, m_inner is 0. > Architecturally, the worst thing here is the knowledge in > RenderContainer::updatePseudoChild that the parent is special when it's a > RenderButton. That needs a comment at least. And we should see what we can do > to avoid this new slightly-fragile linkage between the classes. Even having a > virtual function that was named something cumbersome like > "updatesPseudoChildForChildren" might be better. I would just like to mention that there is already one virtual function that uniquely identifies buttons, and it's called allowsReusingAnonymousChild() :-).
Comment on attachment 9019 [details] Possible fix, no test or change log yet Gist of the fix looks right. I would actually recommend reusing the virtual "allowsReusingAnonymousChild" for this code. It has the advantage of being slightly more generic (since I expect the select popup control may run into this issue too) and entirely appropriate. Then you don't need the new isButton method at all.
Ah, I should have read the comments (just jumped right to the patch from the review queue). :)
Created attachment 9065 [details] Updated patch, including change log and test I unified !allowsReusingAnonymousChild() with isButton() and renamed it createsAnonymousWrapper(). Also renamed doUpdatePseudoChild() to updatePseudoChildForObject() but I'll welcome other suggestions.
Comment on attachment 9065 [details] Updated patch, including change log and test Looks good. r=me Perhaps we can think of an even better name for the updatePseudoChildForObject function some day, but this seems good.
Committed revision 15079.