Bug 43457 - Make the cascade level of "user" styles configurable
Summary: Make the cascade level of "user" styles configurable
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-03 19:11 PDT by Aaron Boodman
Modified: 2010-08-12 15:41 PDT (History)
10 users (show)

See Also:


Attachments
Patch (9.77 KB, patch)
2010-08-04 10:30 PDT, Aaron Boodman
no flags Details | Formatted Diff | Diff
Patch (10.90 KB, patch)
2010-08-04 12:41 PDT, Aaron Boodman
no flags Details | Formatted Diff | Diff
Patch (10.96 KB, patch)
2010-08-10 16:58 PDT, Aaron Boodman
no flags Details | Formatted Diff | Diff
Patch (10.77 KB, patch)
2010-08-11 18:33 PDT, Aaron Boodman
hyatt: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aaron Boodman 2010-08-03 19:11:09 PDT
Chromium needs to set the cascade level to "author" for backward compatibility.
Comment 1 Aaron Boodman 2010-08-04 10:30:20 PDT
Created attachment 63465 [details]
Patch
Comment 2 Eric Seidel (no email) 2010-08-04 10:50:25 PDT
Attachment 63465 [details] did not build on mac:
Build output: http://queues.webkit.org/results/3580800
Comment 3 Aaron Boodman 2010-08-04 12:41:34 PDT
Created attachment 63481 [details]
Patch
Comment 4 Dave Hyatt 2010-08-06 14:31:43 PDT
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.
Comment 5 Aaron Boodman 2010-08-10 16:58:11 PDT
Created attachment 64055 [details]
Patch
Comment 6 Dave Hyatt 2010-08-11 11:49:48 PDT
Comment on attachment 64055 [details]
Patch

r-me
Comment 7 Aaron Boodman 2010-08-11 13:38:29 PDT
Committed r65183: <http://trac.webkit.org/changeset/65183>
Comment 8 WebKit Review Bot 2010-08-11 14:40:57 PDT
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
Comment 9 Kenneth Russell 2010-08-11 17:35:43 PDT
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.
Comment 10 Aaron Boodman 2010-08-11 18:27:40 PDT
Change broke test printing/page-rule-selection.html
Comment 11 Aaron Boodman 2010-08-11 18:33:45 PDT
Created attachment 64179 [details]
Patch
Comment 12 Aaron Boodman 2010-08-11 18:36:04 PDT
The only change to the patch is the addition of " || tempUserStyle->m_pageRuleCount > 0" to line 479 of CSSStyleSelector.cpp.
Comment 13 Dave Hyatt 2010-08-12 14:25:38 PDT
Comment on attachment 64179 [details]
Patch

r=me
Comment 14 Aaron Boodman 2010-08-12 15:41:23 PDT
Committed r65273: <http://trac.webkit.org/changeset/65273>