Bug 209548 - ScopeRuleSets::initializeUserStyle() should not add console logging if there are no injected user style sheets
Summary: ScopeRuleSets::initializeUserStyle() should not add console logging if there ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kate Cheney
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-03-25 10:22 PDT by Kate Cheney
Modified: 2020-03-26 12:00 PDT (History)
3 users (show)

See Also:

Attachments
Patch (1.98 KB, patch)
2020-03-26 08:53 PDT, Kate Cheney 		no flags Details | Formatted Diff | Diff
Patch for landing (1.98 KB, patch)
2020-03-26 11:29 PDT, Kate Cheney 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Kate Cheney 2020-03-25 10:22:03 PDT 
Logging when there are no injected user style sheets is unnecessary and confusing.
Comment 1 Kate Cheney 2020-03-25 10:22:39 PDT 
<rdar://problem/60851745>
Comment 2 Kate Cheney 2020-03-26 08:53:35 PDT 
Created attachment 394616 [details]
Patch
Comment 3 Darin Adler 2020-03-26 10:23:46 PDT 
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.
Comment 4 Kate Cheney 2020-03-26 11:29:08 PDT 
Created attachment 394634 [details]
Patch for landing
Comment 5 Kate Cheney 2020-03-26 11:32:04 PDT 
(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.
Comment 6 Darin Adler 2020-03-26 11:32:45 PDT 
(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.
Comment 7 EWS 2020-03-26 12:00:13 PDT 
Committed r259062: <https://trac.webkit.org/changeset/259062>

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