Bug 87973 - [GTK] Implement disableImageLoading in DRT
Summary: [GTK] Implement disableImageLoading in DRT
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2012-05-31 07:38 PDT by Mario Sanchez Prada
Modified: 2012-07-12 09:01 PDT (History)
4 users (show)

See Also:


Attachments
patch proposal (3.75 KB, patch)
2012-06-13 15:39 PDT, arno.
no flags Details | Formatted Diff | Diff
updated patch (3.83 KB, patch)
2012-06-13 18:32 PDT, arno.
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.