http/tests/security/XFrameOptions/x-frame-options-deny-meta-tag-in-body.html http/tests/security/XFrameOptions/x-frame-options-deny-meta-tag-parent-same-origin-deny.html http/tests/security/XFrameOptions/x-frame-options-deny-meta-tag.html These tests have an extra line in the expected results: http://127.0.0.1:8000/favicon.ico - willSendRequest <NSURLRequest URL http://127.0.0.1:8000/favicon.ico, main document URL http://127.0.0.1:8000/security/XFrameOptions/x-frame-options-deny-meta-tag-in-body.html, http method GET> redirectResponse (null) This is probably innocuous?
Created attachment 55478 [details] Patch
Was going to see about supporting dumpDOMAsWebArchive in order to pass LayoutTests/webarchive/test-link-rel-icon.html too but notice that only Mac supports it. Also noticed that Chromium has: CONSOLE MESSAGE: line 8: Uncaught TypeError: Object [object Object] has no method 'dumpDOMAsWebArchive' layer at (0,0) size 800x600 RenderView at (0,0) size 800x600 layer at (0,0) size 800x600 RenderBlock {HTML} at (0,0) size 800x600 RenderBody {BODY} at (8,8) size 784x584 as the expected results on Win and Linux for this test. So no intention to support the format it seems. Should Qt do the same?
> + * platform/qt/http/tests/security/XFrameOptions/x-frame-options-deny-meta-tag-expected.txt: Added. Qt returns 5 instead of -999. > + * platform/qt/http/tests/security/XFrameOptions/x-frame-options-deny-meta-tag-in-body-expected.txt: Added. Qt returns 5 instead of -999. > + * platform/qt/http/tests/security/XFrameOptions/x-frame-options-deny-meta-tag-parent-same-origin-deny-expected.txt: Added. Qt returns 5 instead of -999. Instead of adding separate test results, you could change FrameLoaderClientQt to rewrite 5 to -999 in the code that prints these callbacks. It should be acceptable as the Windows port seems to be doing a similar thing in their DRT: // Convert the winsock error code to an NSURLError code. if (code == WSAEADDRNOTAVAIL) code = -1004; // NSURLErrorCannotConnectToHose; (In reply to comment #2) > Was going to see about supporting dumpDOMAsWebArchive in order to pass > LayoutTests/webarchive/test-link-rel-icon.html too but notice that only Mac > supports it. Also noticed that Chromium has: > > CONSOLE MESSAGE: line 8: Uncaught TypeError: Object [object Object] has no > method 'dumpDOMAsWebArchive' > layer at (0,0) size 800x600 > RenderView at (0,0) size 800x600 > layer at (0,0) size 800x600 > RenderBlock {HTML} at (0,0) size 800x600 > RenderBody {BODY} at (8,8) size 784x584 > > as the expected results on Win and Linux for this test. So no intention to > support the format it seems. > > Should Qt do the same? I don't think there's a point in running tests for a feature that is not implemented and/or completely disabled. On the other hand, it might at least be useful to make sure that they don't crash.
(In reply to comment #3) > > + * platform/qt/http/tests/security/XFrameOptions/x-frame-options-deny-meta-tag-expected.txt: Added. Qt returns 5 instead of -999. > > + * platform/qt/http/tests/security/XFrameOptions/x-frame-options-deny-meta-tag-in-body-expected.txt: Added. Qt returns 5 instead of -999. > > + * platform/qt/http/tests/security/XFrameOptions/x-frame-options-deny-meta-tag-parent-same-origin-deny-expected.txt: Added. Qt returns 5 instead of -999. > > Instead of adding separate test results, you could change FrameLoaderClientQt to rewrite 5 to -999 in the code that prints these callbacks. It should be acceptable as the Windows port seems to be doing a similar thing in their DRT: > > // Convert the winsock error code to an NSURLError code. > if (code == WSAEADDRNOTAVAIL) > code = -1004; // NSURLErrorCannotConnectToHose; > I think platform-specific results are preferable here. > (In reply to comment #2) > > Was going to see about supporting dumpDOMAsWebArchive in order to pass > > LayoutTests/webarchive/test-link-rel-icon.html too but notice that only Mac > > supports it. Also noticed that Chromium has: > > > > CONSOLE MESSAGE: line 8: Uncaught TypeError: Object [object Object] has no > > method 'dumpDOMAsWebArchive' > > layer at (0,0) size 800x600 > > RenderView at (0,0) size 800x600 > > layer at (0,0) size 800x600 > > RenderBlock {HTML} at (0,0) size 800x600 > > RenderBody {BODY} at (8,8) size 784x584 > > > > as the expected results on Win and Linux for this test. So no intention to > > support the format it seems. > > > > Should Qt do the same? > > I don't think there's a point in running tests for a feature that is not implemented and/or completely disabled. On the other hand, it might at least be useful to make sure that they don't crash. Presumably that's where the chromium guys are coming from. This fix will also unskip LayoutTests/http/tests/misc/favicon-loads-with-images-disabled.html.
Comment on attachment 55478 [details] Patch r- after talking with Robert on irc.
Created attachment 56256 [details] Patch
Committed r59623: <http://trac.webkit.org/changeset/59623>
just a sidenote: it is good when in the changelog there is a match between the "bug title" and the "fix title" [Qt] XFrameOptions/x-frame-options-deny-meta-tag* tests have extra favicon.ico request VS [Qt] Disable Icon Database by default in Qt DRT the later in bugzilla could had catch more attention from others ... you also did not just disabled IconDatabase but added support for image autoloading ... (...) + [Qt] Disable Icon Database by default in Qt DRT + (...) + + https://bugs.webkit.org/show_bug.cgi?id=37382 other than that looks really good to me too
(In reply to comment #8) > just a sidenote: it is good when in the changelog there is a match between the "bug title" and the "fix title" > > [Qt] XFrameOptions/x-frame-options-deny-meta-tag* tests have extra favicon.ico request VS [Qt] Disable Icon Database by default in Qt DRT > > the later in bugzilla could had catch more attention from others ... > > you also did not just disabled IconDatabase but added support for image autoloading ... > > (...) > + [Qt] Disable Icon Database by default in Qt DRT > + > (...) > + > + https://bugs.webkit.org/show_bug.cgi?id=37382 > > other than that looks really good to me too All points taken. Thanks Antonio!