[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


Andreas Kling <kling at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #106917|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #11 from Andreas Kling <kling at webkit.org>  2011-09-10 04:12:20 PST ---
(From update of attachment 106917)
View in context: https://bugs.webkit.org/attachment.cgi?id=106917&action=review

I like where this is going, but let me put my nitpick hat on for a sec.

> Source/WebCore/page/Page.cpp:649
> +    // Allow any local file URI scheme to be loaded

URI here and URL elsewhere? I admit I'm a little foggy on the distinction, but the usage here should be consistent, no?
Also, period at end of sentence.

> Source/WebCore/platform/qt/KURLQt.cpp:46
> +    // Permit valid file or qrc URLs

Ditto.

> Source/WebCore/platform/qt/KURLQt.cpp:47
> +    if (isValid() && (protocolIs("file") || protocolIs("qrc"))) {

I'd prefer isLocalFile() to protocolIs("file") here.
Also, the double occurrence of the "qrc" literal is ugly. Please rephrase this so we only need it once.

> Source/WebCore/platform/qt/KURLQt.cpp:48
> +        // A valid qrc resource path begins with a colon

Ditto.

> Source/WebCore/platform/qt/KURLQt.cpp:52
> +        // Convert the file URL into a proper platform file path

Ditto.

> Source/WebCore/platform/qt/KURLQt.cpp:53
> +        return static_cast<QUrl>(*this).toLocalFile();

Since this line was already changed by a recent patch of yours, this will need a reupload.

> Source/WebKit/qt/tests/qwebpage/tst_qwebpage.cpp:489
> +    QVERIFY(networkManager->requestedUrls.count() >= 1);

Is there an exact value that we could check against instead of ">=1"?

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