Bug 63236

Three more fails: FAIL! : tst_QWebFrame::renderGeometry() Compared values are not the same Actual (size.height()): 100 Expected (picture.boundingRect().height() + frame->scrollBarGeometry(Qt::Horizontal).height()): 0 Loc: [/home/oszi/WebKit/Source/WebKit/qt/tests/qwebframe/tst_qwebframe.cpp(2915)] FAIL! : tst_QWebFrame::renderGeometry() Compared values are not the same Actual (size.width()): 100 Expected (picture.boundingRect().width()): 0 Loc: [/home/oszi/WebKit/Source/WebKit/qt/tests/qwebframe/tst_qwebframe.cpp(2924)] FAIL! : tst_QWebFrame::renderGeometry() Compared values are not the same Actual (size.height()): 100 Expected (picture.boundingRect().height()): 0 Loc: [/home/oszi/WebKit/Source/WebKit/qt/tests/qwebframe/tst_qwebframe.cpp(2927)] Marked as expected fails: http://trac.webkit.org/changeset/89561

It looks like a regression, but I don't know what patch caused this. When you load with substitute data and there are local resources on it, it is no longer emitting loadFinished signal; it might not be trying to load them. In this test, the image inside the inner frame is not being loaded, then the content size remains the same as the frame's size. The expected behavior is to put the image inside, let it expand the content size to 120x120, and then you'll need scrollbars, as the frame size is only 100x100. Without this expansion, the scrollbars are not created, and there's also no painting inside the frame, as there's no content. So in the end, the sizes sum is zero. This is happening only for local resources, like file or qrc protocol, that are supposed to get loaded as soon as the substitute data lands. If you set the substitute data and inside it there's an image that uses a http protocol, for example, then it will try to request it normally. If instead of substitute data, you load the page as a local file (with file protocol), then everything works fine too. This issue is probably causing tst_QWebFrame::setHtmlWithResource to fail as well. It would be really nice to identify what caused this regression.

I think I've finally got what is going wrong. It's a security origin issue. When we load substitute data in that test without defining its base url, it won't have an origin that can load local files (by default only local files can load local files). For some yet unknown reason, we're not enforcing our setting defined in QWebPagePrivate's constructor that we should allow local loads for local files and for substitute data. That's what I'm currently investigating. A quick fix for this test is to put some local file as base url for the iframe, like "file://path/to/local".

Created attachment 99384 [details] We need a proper security origin in order to load local resources, so specified a local file to be its baseUrl. It took me a while, but this patch made this "bug" appears: https://bugs.webkit.org/show_bug.cgi?id=61494. I've explained there how it is happening, and it looks like it didn't affect the other ports. But sincerely I think we've been using setHtml in the wrong way. On QWebPagePrivate's constructor, you can see it's setting a SecurityOrigin policy to allow local files and substitute data to load local resources, but the user of the API is not aware of this (correct me if I'm wrong), and until that patch they were able to do exactly like the test was using. And I've already seen many other circumstances where I would think a certain behavior would happen and then had to specify a valid baseUrl to do it (local storage for example). So if we agree with this current behavior, then it looks like our API docs must reflect this in a better way. The other option to fix this is to correct what was introduced by that patch, but still it looks awkward (for me) that we're doing this little exception here to download local resources, and to everything else we must get a proper security origin. The other bug (https://bugs.webkit.org/show_bug.cgi?id=63235) could also be fixed like this, and I could also add there a case where the user specify the substitute data but do not provide a valid origin, so we can test both behaviors (as the other test is actually testing substitute data with local resources, differently from this one). Waiting for feedback. :)

Comment on attachment 99384 [details] We need a proper security origin in order to load local resources, so specified a local file to be its baseUrl. Good catch.

The commit-queue encountered the following flaky tests while processing attachment 99384 [details]: http/tests/local/formdata/upload-events.html bug 63357 (author: jianli@chromium.org) The commit-queue is continuing to process your patch.

Comment on attachment 99384 [details] We need a proper security origin in order to load local resources, so specified a local file to be its baseUrl. Clearing flags on attachment: 99384 Committed r90181: <http://trac.webkit.org/changeset/90181>

All reviewed patches have been landed. Closing bug.

Revision r90181 cherry-picked into qtwebkit-2.2 with commit 173b2e2 <http://gitorious.org/webkit/qtwebkit/commit/173b2e2>