Bug 34884 – [Qt] QWebSettings::setUserStyleSheetUrl() does not work with windows paths that contain drive letters

• Component: WebKit Qt
• Status: RESOLVED FIXED

[Lev Golod]

When QWebSettings::setUserStyleSheetUrl() is used to set user style sheet nothing happens.
It starts from Qt 4.6.0
Qt 4.5.x works correctly.

To reproduce it's possible to start Qt demo Browser and setup style sheet URL in Edit -> Preferences -> Advanced -> Style Sheet

[Benjamin Poulain]

That seems broken as well with Qt 4.7.

[Antonio Gomes]

I remember diego  playing w/ this stuff. Diegou could you please verify if it is a dup of the bug you solved or a real problem ? if the later, a diagnostic would be great

[Simon Hausmann]

This works for me. Note that you can only specify a path to a local file or a base64 encoded url.

From the documentation:

/*!
    Specifies the location of a user stylesheet to load with every web page.

    The \a location must be either a path on the local filesystem, or a data URL
    with UTF-8 and Base64 encoded data, such as:

    "data:text/css;charset=utf-8;base64,cCB7IGJhY2tncm91bmQtY29sb3I6IHJlZCB9Ow=="

    \sa userStyleSheetUrl()
*/

It seems that
"The location must be either a path on the local filesystem" does not work.

I've tried very hard on 4.6.2 and it just doesn't work.

[Benjamin Poulain]

Could(In reply to comment #4)
> It seems that
> "The location must be either a path on the local filesystem" does not work.
> 
> I've tried very hard on 4.6.2 and it just doesn't work.

Could you please attach a simple test case to reproduce the problem?
So we can quickly check if something is wrong is your code or in WebKit?

[Lev Golod]

#include <QtGui/QApplication>
#include <QWebView>
#include <QUrl>

int main(int argc, char *argv[])
{
    QApplication a(argc, argv);
    QWebView view;
    QByteArray css("div { background-color: red }");
    
    // that's  works fine
    //view.settings()->setUserStyleSheetUrl(QUrl("data:text/css;charset=utf-8;base64," + css.toBase64()));    
    
    // doesn't work
    view.settings()->setUserStyleSheetUrl(QUrl::fromLocalFile("d:/test.css"));    
    
    view.load(QUrl("http://qtsoftware.com/"));
    view.show();
    return a.exec();
}

It seems like 
String KURL::fileSystemPath() const 
returns "/d:/test.css" that QFileInfo can't understand.

[Simon Hausmann]

(In reply to comment #6)
> #include <QtGui/QApplication>
> #include <QWebView>
> #include <QUrl>
> 
> int main(int argc, char *argv[])
> {
>     QApplication a(argc, argv);
>     QWebView view;
>     QByteArray css("div { background-color: red }");
> 
>     // that's  works fine
>    
> //view.settings()->setUserStyleSheetUrl(QUrl("data:text/css;charset=utf-8;base64,"
> + css.toBase64()));    
> 
>     // doesn't work
>     view.settings()->setUserStyleSheetUrl(QUrl::fromLocalFile("d:/test.css"));  
> 
>     view.load(QUrl("http://qtsoftware.com/"));
>     view.show();
>     return a.exec();
> }
> 
> It seems like 
> String KURL::fileSystemPath() const 
> returns "/d:/test.css" that QFileInfo can't understand.

Well spotted. It seems KURL::fileSystemPath() should use toLocalFile() instead of simply path().

[Simon Hausmann]

(In reply to comment #7)
> Well spotted. It seems KURL::fileSystemPath() should use toLocalFile() instead
> of simply path().

Which argh isn't correct either. QUrl::toLocalFile() will return an empty string if the scheme is qrc (or != file && !isEmpty().

Looks like we need a special case and a unit test.

[Kalle Vahlman]

(In reply to comment #8)
> (In reply to comment #7)
> > Well spotted. It seems KURL::fileSystemPath() should use toLocalFile() instead
> > of simply path().
> 
> Which argh isn't correct either. QUrl::toLocalFile() will return an empty
> string if the scheme is qrc (or != file && !isEmpty().
> 
> Looks like we need a special case and a unit test.

For what it's worth, the example doesn't work on Linux either, with a seemingly valid file URL:
 QUrl( "file:///tmp/test.css" )

[Diego Gonzalez]

> For what it's worth, the example doesn't work on Linux either, with a seemingly
> valid file URL:
>  QUrl( "file:///tmp/test.css" )

Actually it works in this case since r53276. Which WebKit revision are you using?

[Kalle Vahlman]

(In reply to comment #10)
> > For what it's worth, the example doesn't work on Linux either, with a seemingly
> > valid file URL:
> >  QUrl( "file:///tmp/test.css" )
> 
> Actually it works in this case since r53276. Which WebKit revision are you
> using?

Apparently something before that, from the 4.6.1 Qt release.

Checking the changeset confirms that I don't have that change here, so sorry for the noise :)

[Jarred Nicholls]

Created attachment 76715 [details]
Proposed patch

QUrl::toLocalFile() is perfectly sufficient right here.  KURL::path() returns a extra leading slash for Windows file URIs, but QUrl::toLocalFile() will handle these well.

A separate bug will be filed for allowing style sheet files as qrc resource URIs, which involves changing WebCore/page/Page.cpp.

[WebKit Review Bot]

Comment on attachment 76715 [details]
Proposed patch

Rejecting attachment 76715 [details] from review queue.

[e-mail] does not have reviewer permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/common/config/committers.py.

- If you do not have reviewer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have reviewer rights please correct the error in WebKitTools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your reviewer rights.

[WebKit Review Bot]

Comment on attachment 76715 [details]
Proposed patch

Rejecting attachment 76715 [details] from commit-queue.

[e-mail] does not have committer permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in WebKitTools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.

[Andreas Kling]

Comment on attachment 76715 [details]
Proposed patch

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

> WebCore/ChangeLog:10
> +        No new tests added.

Why not?

> WebCore/platform/qt/KURLQt.cpp:3
> + * Copyright (C) 2010 Sencha, Inc. All rights reserved.

This change is a bit too skinny to warrant a copyright line. :-)

[Antonio Gomes]

> > WebCore/platform/qt/KURLQt.cpp:3
> > + * Copyright (C) 2010 Sencha, Inc. All rights reserved.
> 
> This change is a bit too skinny to warrant a copyright line. :-)

Although I agree with you, kling, any change can warrant a copyright addition, so it is up to the developer, but definitively he is not wrong.

[Jarred Nicholls]

Created attachment 106905 [details]
Proposed Patch

[Jarred Nicholls]

Created attachment 106915 [details]
Proposed Patch

[Andreas Kling]

Comment on attachment 106915 [details]
Proposed Patch

Great!

[WebKit Review Bot]

Comment on attachment 106915 [details]
Proposed Patch

Clearing flags on attachment: 106915

Committed r94906: <http://trac.webkit.org/changeset/94906>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.