[Regression r96393] Incorrect RenderStyle with the pseudo-element

• Component: CSS
• Status: RESOLVED CONFIGURATION CHANGED

The problem:
Uncorrect style display while using CSS selector attribute with the pseudo-element.
For instance, if the pseudo-elements have styles with some attributes, then only the first found style element will be applied to all the same selector elements on this page.
This issue is relevant to the webkit 535.7+ (Google Chrome 16-17, Safari 5.2 beta)
Example: http://jsfiddle.net/6P3cY/1/


The bug appears in a situation, where an adjacent sibling (+,~) selector or like :first-child haven't been applied.

Temporary fix:
Add any adjacent sibling selector for the element with any style.

Example:
something+a{}
or
something +a{font-size:normal;}

Example with css fix:
http://jsfiddle.net/SyAJG/

Created attachment 129224 [details]
demo

[David Barr]

Created attachment 130282 [details]
Initial reproduction

[David Barr]

This is clearly a regression so I bisected Chromioum builds to try to narrow when it occurred:

You are probably looking for build 103439.
WEBKIT CHANGELOG URL:
http://trac.webkit.org/log/trunk/?rev=96397&stop_rev=96387&verbose=on
CHANGELOG URL:
http://build.chromium.org/f/chromium/perf/dashboard/ui/changelog.html?url=/trunk/src&range=103436:103439
Built at revision:
http://src.chromium.org/viewvc/chrome?view=rev&revision=103439

This looks a likely candidate for causality:
"Universal attribute selectors disable style sharing"
http://trac.webkit.org/changeset/96393

[David Barr]

*** Bug 79719 has been marked as a duplicate of this bug. ***

[David Barr]

Created attachment 130468 [details]
Patch

[David Barr]

It appears that the optimization in r96393 was a little too aggressive, causing a regression for non-global attribute selectors. The attached patch is intended simply as an RFC, I'm sure that it is not as precise as it could be.

[Shezan Baig]

this check could be moved to the end of locateSharedStyle(), since it doesn't need to be done for each call to canShareStyleWithElement(element).  Also, it seems a little too aggresive.  How about something like this at the end of locateSharedStyle:

for (size_t i = 0; i < m_element->attributeCount(); ++i) {
    if (hasSelectorForAttribute(m_element->attributeItem(i)->localName())) {
        return false;
    }
}

[Antti Koivisto]

Comment on attachment 130468 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        This ham-fisted change effectively reverts Antti's effort to
> +        imrove style sharing in the presence of attribute selectors.

Huh?

[Shezan Baig]

the change from comment 7 is a little less aggressive than the attached patch.  however, a better solution would be to add a flag in MatchOptions to not check sub selectors.  This is similar to option (2) in bug 79719, comment 1 except that instead of calling the flag 'ignoreHoverOrActiveState' (which only catches the hover/active subselectors), it can be called something like 'dontCheckSubSelectors' so that it covers the issue in this bug too.  This flag would be set to true when 'locateSharedStyle' does:

    if (matchesRuleSet(m_uncommonAttributeRuleSet.get()))
        return 0;

This flag can be propagated to the SelectorCheckingContext when CSSStyleSelector::checkSelector calls m_checker.checkSelector.  Then, the following check can be added at the beginning of SelectorChecker::checkOneSelector:

    // always match if we want to skip checking subselectors
    if (context.isSubSelector && context.dontCheckSubSelectors)
        return true;

I think this is the ideal change, as it allows for more style sharing than the current patch, and also more than my initial comment 7, while still fixing the bug.  Essentially, we will avoid sharing the style if the attributes match, but without checking subselectors like 'hover,active' (which fixes bug 79719) and also 'before' (which fixes this bug)

[David Barr]

(In reply to comment #8)
> (From update of attachment 130468 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=130468&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        This ham-fisted change effectively reverts Antti's effort to
> > +        imrove style sharing in the presence of attribute selectors.
> 
> Huh?

With hindsight, that summary was far from the best wording. The tone is unintentionally flippant. What I intended to convey was that the proposed change favored correctness over efficiency - to initiate a dialogue about where the cusp of efficiency and correctness lies with respect to attribute selectors and style sharing.

[Brent Fulgham]

Safari, Chrome, and Firefox all agree on rendering for this test case. I don't believe there is any remaining compatibility issue.

[Radar WebKit Bug Importer]

<rdar://problem/96918471>