Bug 170827 – Enough with these favicon.ico errors

• Component: Web Inspector
• Status: NEW

[Dean Jackson]

I'm sick of seeing these errors in the console:
Failed to load resource: the server responded with a status of 404 (Not Found)   https://something/favicon.ico

Especially when the content of the page didn't request it, but the browser did.

At best this could be a warning, maybe with better text? "The browser was unable to find a favicon for this site."

[Radar WebKit Bug Importer]

<rdar://problem/31620816>

[Joseph Pecoraro]

Steps to Reproduce:
1. $ run-safari --no-saved-state
2. Open and inspect about:blank
3. Drag and drop a local simple.html onto Safari
  => Error in console about favicon.ico 404

[Joseph Pecoraro]

Created attachment 307086 [details]
[PATCH] Proposed Fix

I still want to get rid of the file:///favicon.ico but my brain continues to think "maybe this could be legit".

Maybe we should just prevent against this single file URL path, since its so unlikely to be real?

[Build Bot]

Attachment 307086 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/network/ResourceRequestBase.h:155:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Sam Weinig]

Given that you are inspecting what the browser is doing, and it is making a request to your server, it seems like removing it altogether wouldn't be ideal (in fact, there are other requests the browser is making to your server, for other icons, that it would be nice if the inspector showed in some way).

[Sam Weinig]

(In reply to Sam Weinig from comment #5)
> Given that you are inspecting what the browser is doing, and it is making a
> request to your server, it seems like removing it altogether wouldn't be
> ideal (in fact, there are other requests the browser is making to your
> server, for other icons, that it would be nice if the inspector showed in
> some way).

Perhaps just indicating a "soft failure", would be more appropriate?

[Joseph Pecoraro]

(In reply to Sam Weinig from comment #6)
> Perhaps just indicating a "soft failure", would be more appropriate?

This patch converted the console message from an error to a warnings.

The "setHiddenFromInspector(true)" part of the patch hides the resource from the Network/Resources tab. I think its weird that it shows up there when it is not a resource requested by the page, I can add it back.

I see the Web Inspector as page centric, not browser centric. You can have two separate pages being inspected, and they don't show each others traffic. That said, I would see the benefit of showing some actions the browser takes on behalf of the page. In this case if the page explicitly set a favicon path it would make sense to show details about the browser's request for the icon.

[Build Bot]

Comment on attachment 307086 [details]
[PATCH] Proposed Fix

Attachment 307086 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3536683

New failing tests:
webrtc/multi-video.html

[Build Bot]

Created attachment 307178 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[Alex Christensen]

Comment on attachment 307086 [details]
[PATCH] Proposed Fix

This patch feels bad.  We're passing a loader around that is mostly nullptr except one place.  The layering feels wrong but I'm not sure how to describe what would be better.  This is a very specific case we want to ignore, but we seem to be ignoring errors from ALL icon loads.  Also, there are no tests.