WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
63236
[Qt] Fix tst_QWebFrame::renderGeometry() API test
https://bugs.webkit.org/show_bug.cgi?id=63236
Summary
[Qt] Fix tst_QWebFrame::renderGeometry() API test
Csaba Osztrogonác
Reported
2011-06-23 01:55:08 PDT
http://build.webkit.org/builders/Qt%20Linux%20Release/builds/34519
FAIL! : tst_QWebFrame::renderGeometry() Compared values are not the same Actual (size.width()): 100 Expected (picture.boundingRect().width() + frame->scrollBarGeometry(Qt::Vertical).width()): 0 Loc: [/ramdisk/qt-linux-release/build/Source/WebKit/qt/tests/qwebframe/tst_qwebframe.cpp(2912)]
Attachments
We need a proper security origin in order to load local resources, so specified a local file to be its baseUrl.
(2.65 KB, patch)
2011-06-30 15:57 PDT
,
Rafael Brandao
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Csaba Osztrogonác
Comment 1
2011-06-23 05:51:43 PDT
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
Rafael Brandao
Comment 2
2011-06-28 09:27:09 PDT
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.
Rafael Brandao
Comment 3
2011-06-29 18:00:24 PDT
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".
Rafael Brandao
Comment 4
2011-06-30 15:57:26 PDT
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. :)
Benjamin Poulain
Comment 5
2011-06-30 16:04:49 PDT
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.
WebKit Review Bot
Comment 6
2011-06-30 16:29:00 PDT
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.
WebKit Review Bot
Comment 7
2011-06-30 16:30:30 PDT
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
>
WebKit Review Bot
Comment 8
2011-06-30 16:30:36 PDT
All reviewed patches have been landed. Closing bug.
Ademar Reis
Comment 9
2011-07-01 07:47:15 PDT
Revision
r90181
cherry-picked into qtwebkit-2.2 with commit 173b2e2 <
http://gitorious.org/webkit/qtwebkit/commit/173b2e2
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug