Bug 170196 - Web Inspector: new icon for Disable Caches button in Network Tab
Summary: Web Inspector: new icon for Disable Caches button in Network Tab
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Jon Davis
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-03-28 13:54 PDT by BJ Burg
Modified: 2017-04-17 16:16 PDT (History)
11 users (show)

See Also:


Attachments
Patch (9.13 KB, patch)
2017-04-14 14:40 PDT, Jon Davis
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews106 for mac-elcapitan-wk2 (1.10 MB, application/zip)
2017-04-14 15:50 PDT, Build Bot
no flags Details
Patch (10.50 KB, patch)
2017-04-17 10:37 PDT, Jon Davis
no flags Details | Formatted Diff | Diff
Before (above) and after (below) screenshot (41.53 KB, image/png)
2017-04-17 10:38 PDT, Jon Davis
no flags Details
Patch (10.54 KB, patch)
2017-04-17 14:20 PDT, Jon Davis
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 (1.08 MB, application/zip)
2017-04-17 15:15 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description BJ Burg 2017-03-28 13:54:25 PDT
Going to investigate this as a followup.
Comment 1 Radar WebKit Bug Importer 2017-03-28 13:54:54 PDT
<rdar://problem/31306389>
Comment 2 Jon Davis 2017-04-14 14:40:06 PDT
Created attachment 307147 [details]
Patch
Comment 3 Joseph Pecoraro 2017-04-14 15:33:33 PDT
Comment on attachment 307147 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/NetworkGridContentView.js:135
> -            this._disableResourceCacheNavigationItem = new WebInspector.ActivateButtonNavigationItem("disable-resource-cache", toolTipForDisableResourceCache, activatedToolTipForDisableResourceCache, "Images/StepOver.svg", 16, 16);
> +            this._disableResourceCacheNavigationItem = new WebInspector.ActivateButtonNavigationItem("disable-resource-cache", toolTipForDisableResourceCache, activatedToolTipForDisableResourceCache, "Images/IgnoreCaches.svg", 16, 16);

You should do something for gtk, and file a bug on them.

Would probably be okay to fallback to StepOver.svg on gtk.
Comment 4 Matt Baker 2017-04-14 15:36:31 PDT
Comment on attachment 307147 [details]
Patch

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

r=me, address Joe's comment and also mention the new artwork in the change log (only the update to the Clear button is mentioned).

In general for changes like this a before/after screen shot for changing artwork is nice.

> Source/WebInspectorUI/ChangeLog:11
> +        * UserInterface/Images/IgnoreCaches.svg: Added.

I think this should be singular: IgnoreCache.svg
Comment 5 Build Bot 2017-04-14 15:50:56 PDT
Comment on attachment 307147 [details]
Patch

Attachment 307147 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3535872

New failing tests:
webrtc/multi-video.html
Comment 6 Build Bot 2017-04-14 15:50:57 PDT
Created attachment 307160 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 7 Nikita Vasilyev 2017-04-14 19:47:05 PDT
Comment on attachment 307147 [details]
Patch

Unrelated test failure. Setting cq+.
Comment 8 Joseph Pecoraro 2017-04-14 19:52:12 PDT
Comment on attachment 307147 [details]
Patch

I have comments that I think need to be addressed.
Comment 9 Jon Davis 2017-04-17 08:45:56 PDT
I'll address the comments. Thanks for the feedback.
Comment 10 Jon Davis 2017-04-17 10:37:45 PDT
Created attachment 307277 [details]
Patch
Comment 11 Jon Davis 2017-04-17 10:38:55 PDT
Created attachment 307278 [details]
Before (above) and after (below) screenshot
Comment 12 Joseph Pecoraro 2017-04-17 11:36:36 PDT
Comment on attachment 307277 [details]
Patch

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

r=me

> Source/WebInspectorUI/ChangeLog:12
> +        Used GTK's StepOver.svg for fallback art for GTK.

I think you should still file a bugzilla bug on GTK. There is someone working on images right now that might jump on this. People to CC would be the list on:
https://bugs.webkit.org/show_bug.cgi?id=153892

> Source/WebInspectorUI/UserInterface/Images/gtk/IgnoreCaches.svg:5
> + <path d="m0.66428 7.6325 1.4142 1.4142 4.794-4.794c0.59416-0.59416 1.5272-0.59416 2.1213 0l2.8947 2.8947h-1.812c-0.26372 0.0054-0.54756 0.10561-0.7292 0.28726l-0.70711 0.70711 0.70711 0.70711c0.21881 0.21881 0.4544 0.3267 0.7292 0.33146l5.2149-0.0442 0.04419-5.2149c-0.0048-0.2748-0.11264-0.51039-0.33146-0.7292l-0.7071-0.7071-0.70711 0.70711c-0.18165 0.18165-0.28187 0.46548-0.28726 0.7292v1.812l-2.8947-2.8947c-1.3585-1.3585-3.5913-1.3585-4.9497 0z"/>

I'd have expected fill="currentColor" on GTK since this button is enabled/disabled. Right now I think it would always look disabled even when activated.
Comment 13 Jon Davis 2017-04-17 14:01:12 PDT
I filed https://bugs.webkit.org/show_bug.cgi?id=170914 for GTK
Comment 14 Jon Davis 2017-04-17 14:20:04 PDT
Created attachment 307295 [details]
Patch
Comment 15 Build Bot 2017-04-17 15:15:19 PDT
Comment on attachment 307295 [details]
Patch

Attachment 307295 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3553213

New failing tests:
webrtc/multi-video.html
Comment 16 Build Bot 2017-04-17 15:15:20 PDT
Created attachment 307305 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 17 Joseph Pecoraro 2017-04-17 15:46:51 PDT
Comment on attachment 307305 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

Unrelated bot failures.
Comment 18 WebKit Commit Bot 2017-04-17 16:16:48 PDT
Comment on attachment 307295 [details]
Patch

Clearing flags on attachment: 307295

Committed r215440: <http://trac.webkit.org/changeset/215440>
Comment 19 WebKit Commit Bot 2017-04-17 16:16:49 PDT
All reviewed patches have been landed.  Closing bug.