Bug 43457 – Make the cascade level of "user" styles configurable

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Aaron Boodman]

Chromium needs to set the cascade level to "author" for backward compatibility.

[Aaron Boodman]

Created attachment 63465 [details]
Patch

[Eric Seidel (no email)]

Attachment 63465 [details] did not build on mac:
Build output: http://queues.webkit.org/results/3580800

[Aaron Boodman]

Created attachment 63481 [details]
Patch

[Dave Hyatt]

Comment on attachment 63481 [details]
Patch


> +    {
> +        OwnPtr<CSSRuleSet> tempUserStyle(new CSSRuleSet);
>          if (pageUserSheet)
> -            m_userStyle->addRulesFromSheet(pageUserSheet, *m_medium, this);
> +            tempUserStyle->addRulesFromSheet(pageUserSheet, *m_medium, this);
>          if (pageGroupUserSheets) {
>              unsigned length = pageGroupUserSheets->size();
> -            for (unsigned i = 0; i < length; i++)
> -                m_userStyle->addRulesFromSheet(pageGroupUserSheets->at(i).get(), *m_medium, this);
> +            for (unsigned i = 0; i < length; i++) {
> +                if (pageGroupUserSheets->at(i)->isUserStyleSheet())
> +                    tempUserStyle->addRulesFromSheet(pageGroupUserSheets->at(i).get(), *m_medium, this);
> +                else
> +                    m_authorStyle->addRulesFromSheet(pageGroupUserSheets->at(i).get(), *m_medium, this);
> +            }
>          }
> +
> +        if (tempUserStyle->m_ruleCount > 0)
> +            m_userStyle = tempUserStyle.leakPtr();
>      }

Unless I'm misreading this, you have indented code inside braces but no longer have an if statement?  Just pull the code out of the braces.


> -            parsedSheet->setIsUserStyleSheet(true);
> +
> +            if (sheet->level() == UserStyleSheet::UserLevel)
> +                parsedSheet->setIsUserStyleSheet(true);
> +            else {
> +                ASSERT(sheet->level() == UserStyleSheet::AuthorLevel);
> +                parsedSheet->setIsUserStyleSheet(false);
> +            }
> +

I wouldn't worry about the assert here... just do:

parsedSheet->setIsUserStyleSheet(sheet->level() == UserStyleSheet::UserLevel)

More compact.


>      OwnPtr<UserStyleSheet> userStyleSheet(new UserStyleSheet(source, url, whitelist, blacklist, injectedFrames));
> +    userStyleSheet->setLevel(level);

Just make level part of the constructor, so that you don't have to have a 2nd line with a setLevel call.

Everything else looks fine.

[Aaron Boodman]

Created attachment 64055 [details]
Patch

[Dave Hyatt]

Comment on attachment 64055 [details]
Patch

r-me

[Aaron Boodman]

Committed r65183: <http://trac.webkit.org/changeset/65183>

[WebKit Review Bot]

http://trac.webkit.org/changeset/65183 might have broken Leopard Intel Release (Tests)
The following changes are on the blame list:
http://trac.webkit.org/changeset/65182
http://trac.webkit.org/changeset/65183

[Kenneth Russell]

I think there may have been a merge problem in WebCore/ChangeLog with one of the patches landed for this bug. See https://bugs.webkit.org/show_bug.cgi?id=40320 and http://trac.webkit.org/changeset/65193 which it references.

[Aaron Boodman]

Change broke test printing/page-rule-selection.html

[Aaron Boodman]

Created attachment 64179 [details]
Patch

[Aaron Boodman]

The only change to the patch is the addition of " || tempUserStyle->m_pageRuleCount > 0" to line 479 of CSSStyleSelector.cpp.

[Dave Hyatt]

Comment on attachment 64179 [details]
Patch

r=me

[Aaron Boodman]

Committed r65273: <http://trac.webkit.org/changeset/65273>