bookmark is invisible if favicon.ico is not found

[luixxiul]

Describe the issue you encountered: bookmark is invisible in "show only favicon mode" if favicon.ico is not found (404)

Expected behavior: a class should be added so that it can be styled

• Brave Version: Master Branch
• Screenshot if needed:
  
• Any related issues: Show only favicons #1739, Bookmarks overflow in "show only favicon" mode #2073

[bbondy]

Maybe show a generic file icon in this case?

[luixxiul]

or border: 1px dotted?

[bbondy]

@bradleyrichter for final say

[bsclifton]

This is a tough one... even if you're in the regular bookmarks toolbar view, it happens (just 16 pixels of dead space). I like the idea of showing a generic icon

The hard part of this defect is... it throws a 404 at the CSS level when the background-image: url(); part evaluates. It'll show the 404 in the JavaScript console, but I'm not sure how to intercept this. The ideal solution would be to intercept this, remove the background image CSS attribute and then add the classes "fa fa-file-o"

Any ideas on how to catch that? I put an onerror="" handler on the span owning the style that 404s, but it doesn't get executed.

[bradleyrichter]

We should follow Chrome or FX lead on this one.

On Jun 4, 2016, at 8:16 AM, Brian Clifton notifications@github.com wrote:

This is a tough one... even if you're in the regular bookmarks toolbar view, it happens (just 16 pixels of dead space). I like the idea of showing a generic icon

The hard part of this defect is... it throws a 404 at the CSS level when the background-image: url(); part evaluates. It'll show the 404 in the JavaScript console, but I'm not sure how to intercept this. The ideal solution would be to intercept this, remove the background image CSS attribute and then add the classes "fa fa-file-o"

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[cezaraugusto]

@bradleyrichter Chrome sets its own "paper" icon for bookmarks. Can we have one as well?

[bradleyrichter]

yes please.

FA file-o

[luixxiul]

Test plan: #5826 (comment)