Bug 87973 – [GTK] Implement disableImageLoading in DRT

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Mario Sanchez Prada]

http/tests/misc/favicon-loads-with-images-disabled.html seems to be failing because of that reason

[Mario Sanchez Prada]

Forgot to attach the diff:

--- /home/slave/webkitgtk/gtk-linux-64-debug/build/layout-test-results/http/tests/misc/favicon-loads-with-images-disabled-expected.txt 
+++ /home/slave/webkitgtk/gtk-linux-64-debug/build/layout-test-results/http/tests/misc/favicon-loads-with-images-disabled-actual.txt 
@@ -1,6 +1,7 @@
 <unknown> - didFinishLoading
 http://127.0.0.1:8000/misc/favicon-loads-with-images-disabled.html - willSendRequest <NSURLRequest URL http://127.0.0.1:8000/misc/favicon-loads-with-images-disabled.html, main document URL http://127.0.0.1:8000/misc/favicon-loads-with-images-disabled.html, http method GET> redirectResponse (null)
 http://127.0.0.1:8000/misc/favicon-loads-with-images-disabled.html - didReceiveResponse <NSURLResponse http://127.0.0.1:8000/misc/favicon-loads-with-images-disabled.html, http status code 200>
+http://127.0.0.1:8000/misc/resources/favicon.ico - willSendRequest <NSURLRequest URL http://127.0.0.1:8000/misc/resources/favicon.ico, main document URL http://127.0.0.1:8000/misc/favicon-loads-with-images-disabled.html, http method GET> redirectResponse (null)
 Radar 6973106 and https://bugs.webkit.org/show_bug.cgi?id=27896 - Favicons still load when automatic image loading is disabled.
 This test uses DRT's resource load delegate callback mode to see if the favicon is loaded even when image loading is off.

[Mario Sanchez Prada]

Added to test_expectations.txt: http://trac.webkit.org/changeset/119101

[arno.]

Created attachment 147428 [details]
patch proposal

If disableImageLoading is implemented, then favicon-loads-with-icon-loading-override.html won't work anymore. It is only "working" because disableImageLoading was not implemented

[Martin Robinson]

Comment on attachment 147428 [details]
patch proposal

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

Seems sane though there are a few things that look off:

> Tools/DumpRenderTree/gtk/LayoutTestControllerGtk.cpp:453
>      // FIXME: Implement for testing fix for https://bugs.webkit.org/show_bug.cgi?id=27896
>      // Also need to make sure image loading is re-enabled for each new test.

It'd make sense to remove this comment now.

> LayoutTests/ChangeLog:6
> +        [GTK] Implement disableImageLoading in DRT
> +        Add TEXT expectation to favicon-loads-with-icon-loading-override.html
> +        which was only working by accident.
> +        https://bugs.webkit.org/show_bug.cgi?id=87973

Your changelog looks a little funky here. The description should go below the "Reviewed by" line.

> LayoutTests/platform/gtk/TestExpectations:1293
> +BUGWKGTK   : http/tests/misc/favicon-loads-with-icon-loading-override.html = TEXT

It seems you are replacing an existing failure with another?

[arno.]

(In reply to comment #4)

> > LayoutTests/platform/gtk/TestExpectations:1293
> > +BUGWKGTK   : http/tests/misc/favicon-loads-with-icon-loading-override.html = TEXT
> 
> It seems you are replacing an existing failure with another?

Actually favicon-loads-with-icon-loading-override.html tests a special case:
a pref to load of favicons even when image loading is disabled.
So, I'm replacing a test succeeding but testing nothing by a test testing an actual behaviour

[arno.]

Created attachment 147461 [details]
updated patch

patch fixing style issues

[WebKit Review Bot]

Comment on attachment 147461 [details]
updated patch

Clearing flags on attachment: 147461

Committed r122467: <http://trac.webkit.org/changeset/122467>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.