[Webkit-unassigned] [Bug 51159] [Qt] Permit qrc resources to load in QWebSettings::setUserStyleSheetUrl()

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=51159





--- Comment #25 from Jarred Nicholls <jarred at sencha.com>  2011-09-13 12:01:55 PST ---
(In reply to comment #24)
> (In reply to comment #22)
> > (From update of attachment 107204 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=107204&action=review
> > 
> > > Source/WebCore/page/Page.cpp:689
> > > +    // Allow any local file URL scheme to be loaded.
> > > +    if (SchemeRegistry::shouldTreatURLSchemeAsLocal(url.protocol()))
> > 
> > I wonder if this should be a separate change with tests? Especially since it is in shared code
> 
> Unfortunately it can't be a separate change.  I think the current user style sheet tests, with the addition of these qrc tests, still cover this functionality.
> 
> > 
> > > Source/WebCore/platform/qt/KURLQt.cpp:54
> > > +    if (isValid()) {
> > > +        if (isLocalFile())
> > > +            return static_cast<QUrl>(*this).toLocalFile();
> > > +            
> > > +        // A valid qrc resource path begins with a colon.
> > > +        if (protocolIs("qrc"))
> > > +            return ":" + path();
> > > +    }
> > 
> > We generally try avoiding indentation when possible. So what about
> > 
> > if (!isValid)
> >     return String();
> 
> Yes this would be better, thanks.

On second thought, since we can't guarantee the protocol is either file or qrc, we'd still need the second return String() at the bottom and I was avoiding that...seems redundant I suppose.  But I understand the indentation.  What'cha think?

Thanks for the review.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.