Bug 209548 – ScopeRuleSets::initializeUserStyle() should not add console logging if there are no injected user style sheets

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Kate Cheney]

Logging when there are no injected user style sheets is unnecessary and confusing.

[Kate Cheney]

<rdar://problem/60851745>

[Kate Cheney]

Created attachment 394616 [details]
Patch

[Darin Adler]

Comment on attachment 394616 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +       ScopeRuleSets::initializeUserStyle() should not add console logging if there are no injected user style sheets
> +       https://bugs.webkit.org/show_bug.cgi?id=209548
> +       <rdar://problem/60851745>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Logging when there are no injected user style sheets is unnecessary and confusing.
> +
> +        * style/StyleScopeRuleSets.cpp:
> +        (WebCore::Style::ScopeRuleSets::initializeUserStyle):

Strange indenting

> Source/WebCore/style/StyleScopeRuleSets.cpp:96
> -    if (page && page->mainFrame().loader().client().hasNavigatedAwayFromAppBoundDomain())
> +    if (page && page->mainFrame().loader().client().hasNavigatedAwayFromAppBoundDomain() && !extensionStyleSheets.injectedUserStyleSheets().isEmpty())
>          m_styleResolver.document().addConsoleMessage(MessageSource::Security, MessageLevel::Warning, "Ignoring user style sheet for non-app bound domain."_s);

Besides not logging, this now means we’ll call collectRulesFromUserStyleSheets on the empty vector. No harm to that I suppose.

[Kate Cheney]

Created attachment 394634 [details]
Patch for landing

[Kate Cheney]

(In reply to Darin Adler from comment #3)
> Comment on attachment 394616 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=394616&action=review
> 

Thanks Darin!

> > Source/WebCore/ChangeLog:12
> > +       ScopeRuleSets::initializeUserStyle() should not add console logging if there are no injected user style sheets
> > +       https://bugs.webkit.org/show_bug.cgi?id=209548
> > +       <rdar://problem/60851745>
> > +
> > +        Reviewed by NOBODY (OOPS!).
> > +
> > +        Logging when there are no injected user style sheets is unnecessary and confusing.
> > +
> > +        * style/StyleScopeRuleSets.cpp:
> > +        (WebCore::Style::ScopeRuleSets::initializeUserStyle):
> 
> Strange indenting

Good catch, fixed this in the patch for landing.

> 
> > Source/WebCore/style/StyleScopeRuleSets.cpp:96
> > -    if (page && page->mainFrame().loader().client().hasNavigatedAwayFromAppBoundDomain())
> > +    if (page && page->mainFrame().loader().client().hasNavigatedAwayFromAppBoundDomain() && !extensionStyleSheets.injectedUserStyleSheets().isEmpty())
> >          m_styleResolver.document().addConsoleMessage(MessageSource::Security, MessageLevel::Warning, "Ignoring user style sheet for non-app bound domain."_s);
> 
> Besides not logging, this now means we’ll call
> collectRulesFromUserStyleSheets on the empty vector. No harm to that I
> suppose.

I could follow up and add an if statement to check if extensionStyleSheets.injectedUserStyleSheets() is empty. I would probably also want to add one for extensionStyleSheets.documentUserStyleSheets() to be consistent.

[Darin Adler]

(In reply to katherine_cheney from comment #5)
> I could follow up and add an if statement to check if
> extensionStyleSheets.injectedUserStyleSheets() is empty. I would probably
> also want to add one for extensionStyleSheets.documentUserStyleSheets() to
> be consistent.

You should only do that if there’s a good reason. Otherwise I would suggest leaving things as-is.

[EWS]

Committed r259062: <https://trac.webkit.org/changeset/259062>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 394634 [details].