Bug 133934 – Subtrees with :first-child and :last-child are not invalidated when siblings are added/removed

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Benjamin Poulain]

Subtrees with :first-child and :last-child are not invalidated when siblings are added/removed

[Benjamin Poulain]

Created attachment 233150 [details]
Patch

[Antti Koivisto]

Comment on attachment 233150 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=233150&action=review

> Source/WebCore/dom/Element.cpp:1552
> +    RenderStyle* style = element.renderStyle();
> +    if (!style || (element.styleAffectedByEmpty() && (!style->emptyState() || element.hasChildNodes())))
> +        element.setNeedsStyleRecalc();
>  }

Isn't this a bit blunt? Any element with display:none is going to have it's style recomputed on child change whether or not there are any :empty rules around.

[Benjamin Poulain]

Created attachment 233170 [details]
Patch

[Build Bot]

Comment on attachment 233170 [details]
Patch

Attachment 233170 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6303961720029184

New failing tests:
http/tests/misc/acid3.html

[Build Bot]

Created attachment 233175 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Benjamin Poulain]

Comment on attachment 233170 [details]
Patch

Damn, Acid 3 does not like the :empty change...

[Benjamin Poulain]

Arg, of course, my last change is non-sense. There can be non-element nodes as children, that still qualify as :empty.

[Benjamin Poulain]

Created attachment 233191 [details]
Patch

[Antti Koivisto]

Comment on attachment 233191 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=233191&action=review

> Source/WebCore/dom/Element.cpp:1597
> +            if (!style || style->lastChildState())

I'm still slightly worried we are invalidating more in display:none case. However maybe this doesn't have much real world significance.

[Benjamin Poulain]

Comment on attachment 233191 [details]
Patch

Clearing flags on attachment: 233191

Committed r170121: <http://trac.webkit.org/changeset/170121>

[Benjamin Poulain]

All reviewed patches have been landed.  Closing bug.