Bug 87973

Summary: [GTK] Implement disableImageLoading in DRT
Product: WebKit Reporter: Mario Sanchez Prada <mario>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: a.renevier, mrobinson, svillar, webkit.review.bot
Priority: P2 Keywords: Gtk
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch proposal
none
updated patch none

Description Mario Sanchez Prada 2012-05-31 07:38:56 PDT
http/tests/misc/favicon-loads-with-images-disabled.html seems to be failing because of that reason
Comment 1 Mario Sanchez Prada 2012-05-31 07:39:53 PDT
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.
Comment 2 Mario Sanchez Prada 2012-05-31 07:47:10 PDT
Added to test_expectations.txt: http://trac.webkit.org/changeset/119101
Comment 3 arno. 2012-06-13 15:39:47 PDT
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
Comment 4 Martin Robinson 2012-06-13 18:16:20 PDT
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?
Comment 5 arno. 2012-06-13 18:31:08 PDT
(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
Comment 6 arno. 2012-06-13 18:32:34 PDT
Created attachment 147461 [details]
updated patch

patch fixing style issues
Comment 7 WebKit Review Bot 2012-07-12 09:01:21 PDT
Comment on attachment 147461 [details]
updated patch

Clearing flags on attachment: 147461

Committed r122467: <http://trac.webkit.org/changeset/122467>
Comment 8 WebKit Review Bot 2012-07-12 09:01:26 PDT
All reviewed patches have been landed.  Closing bug.