RESOLVED FIXED 27512
Test for QWebFrame::hasSetFocus() is trying to use a not defined resource.
https://bugs.webkit.org/show_bug.cgi?id=27512
Summary Test for QWebFrame::hasSetFocus() is trying to use a not defined resource.
Andre Pedralho
Reported Tuesday, July 21, 2009 9:18:27 PM UTC
There is no frametest folder and qwebframe.qrc does not specify where to find it. tst_qwebframe.cpp: void tst_QWebFrame::hasSetFocus() { (...) QUrl url = QUrl("qrc:///frametest/iframe.html"); _page->mainFrame()->load(url); (...) } qwebframe.qrc: <!DOCTYPE RCC><RCC version="1.0"> <qresource prefix="/"> <file>image.png</file> <file>style.css</file> </qresource> </RCC>
Attachments
Copied resources from qwebpage tests and added the references to the qwebframe.qrc. (2.31 KB, patch)
2009-07-21 13:38 PDT, Andre Pedralho
manyoso: review-
Using setHtml methods to define the test web page instead of external resources. (2.70 KB, patch)
2009-07-24 07:48 PDT, Andre Pedralho
no flags
Using QSignalSpy to control whether a load finished signal has been emited instead of just wait for some miliseconds. (2.42 KB, patch)
2009-07-27 11:39 PDT, Andre Pedralho
hausmann: review+
Andre Pedralho
Comment 1 Tuesday, July 21, 2009 9:38:14 PM UTC
Created attachment 33207 [details] Copied resources from qwebpage tests and added the references to the qwebframe.qrc.
Antonio Gomes
Comment 2 Tuesday, July 21, 2009 9:51:37 PM UTC
we are duping the resources used in qwebframe and qwebpage. should not we make them to share the resource ?
Adam Treat
Comment 3 Wednesday, July 22, 2009 2:35:39 PM UTC
Comment on attachment 33207 [details] Copied resources from qwebpage tests and added the references to the qwebframe.qrc. As tonikitoo pointed out, we shouldn't dupe the resources, but rather share them.
Andre Pedralho
Comment 4 Wednesday, July 22, 2009 7:06:42 PM UTC
(In reply to comment #3) > (From update of attachment 33207 [details]) > As tonikitoo pointed out, we shouldn't dupe the resources, but rather share > them. Agree with that, however "the listed resource files must be located in the same directory as the .qrc file, or one of its subdirectories" according to the Qt resources docs. As there are two different directories to QWebFrame and to QWebPage tests the only way to share the resources would be to have a pool of resources in WebKit/qt/tests/'resources'/ with the qrc files related to both tests. I mean: ls WebKit/qt/tests/: benchmarks qwebelement qwebframe (...) qwebpage qwebview RESOURCES tests.pro ls WebKit/qt/tests/resources: FRAMETEST image.png qwebframe.qrc style.css tst_qwebpage.qrc IMHO, the best solution here is to create separated resources to each tests, maybe duplicating it.
Andre Pedralho
Comment 5 Thursday, July 23, 2009 2:06:21 PM UTC
Hausmann, we need your opinion to find the best solution here. Can you help us?
Simon Hausmann
Comment 6 Thursday, July 23, 2009 9:26:01 PM UTC
Hmmm, another option, potentially simpler, would be to replace the use of the resources for html with data urls. What do you think?
Antonio Gomes
Comment 7 Thursday, July 23, 2009 9:30:47 PM UTC
simon, he has done so for this renderContents removal patch (not uploaded yet) ... and it looks pretty nice. and it would work here too, imho ps: we should "invalid" this one instead of mark as "fixed" ?
Andre Pedralho
Comment 8 Friday, July 24, 2009 3:46:45 PM UTC
Reopening as the bug itself still there and might be fixed with the solution proposed in comment #6.
Andre Pedralho
Comment 9 Friday, July 24, 2009 3:48:24 PM UTC
Created attachment 33445 [details] Using setHtml methods to define the test web page instead of external resources.
Andre Pedralho
Comment 10 Monday, July 27, 2009 7:39:18 PM UTC
Created attachment 33560 [details] Using QSignalSpy to control whether a load finished signal has been emited instead of just wait for some miliseconds. Marking old patch obsolete as per suggestions in IRC.
Simon Hausmann
Comment 11 Tuesday, July 28, 2009 2:01:54 PM UTC
Comment on attachment 33560 [details] Using QSignalSpy to control whether a load finished signal has been emited instead of just wait for some miliseconds. r=me. Thanks
Simon Hausmann
Comment 12 Tuesday, July 28, 2009 2:04:51 PM UTC
Landed in r46475
Note You need to log in before you can comment on or make changes to this bug.