Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

Failure to load subtitle link favicon using /proxy?url #4738

Closed
punamdahiya opened this issue Aug 3, 2018 · 6 comments
Closed

Failure to load subtitle link favicon using /proxy?url #4738

punamdahiya opened this issue Aug 3, 2018 · 6 comments
Assignees

Comments

@punamdahiya
Copy link
Contributor

punamdahiya commented Aug 3, 2018

[Steps to reproduce]:
- Save a shot
- Paste the saved shot's link in Chrome.
- Console shows message Failed to load resource: the server responded with a status of 404 (Not Found)

@punamdahiya
Copy link
Contributor Author

console-error-chrome

@punamdahiya
Copy link
Contributor Author

Issue reported in #4710 could be fall out of this error in Google Chrome
#4710 (comment)
Also, This call seems to be redundant as favicon for subtitle link is hidden #4710 (comment)

@punamdahiya punamdahiya self-assigned this Aug 3, 2018
@jaredhirsch
Copy link
Member

Looks like createProxyUrl (in proxy-url.js, called when creating the shot page) assumes a regular https URL for a favicon, while we are saving the favicon as a data-URI.

Indeed, if you look at the data submitted in the PUT request when creating a new shot, the favicon is indeed saved as a base64 image, rather than, say, https://facebook.com/favicon.ico:

screen shot 2018-08-03 at 3 35 06 pm

@jaredhirsch
Copy link
Member

When the shot is created in the background page, we get the favicon URL from Tabs.tab.favIconUrl:

https://github.com/mozilla-services/screenshots/blob/master/addon/webextension/background/takeshot.js#L13

screen shot 2018-08-03 at 3 45 35 pm

Apparently this API now returns a data-uri instead of the URL of the original favicon.

@jaredhirsch
Copy link
Member

Ah yeah, it turns out that the addons code just gets the favicon from tabbrowser (deeper code in Firefox), and some favicon refactoring there resulted in the image itself, not its URL, being returned by tabs.Tab.favIconUrl.

This is definitely an upstream bug; I'll update this issue when I get a BMO link for the Firefox bug to fix it.

In the meantime, we should just stop loading the favicon on the server pages altogether.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants