-
Notifications
You must be signed in to change notification settings - Fork 910
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Favicon Failed to get image from website, Found Fix #821
Comments
I really wish I had seen this sooner as I have wasted HOURS on this darn issue which still plagues numerous commits all the way through today's (12/12) and I believe has rendered the entire TGS broken again without the ability to even initialize at this point. What commit are you currently running now have you updated since 12/4 build? I ask because despite this fix never been put in, with all this refactoring the errors are like a moving target and now all the code has been moved to different files (gsFavion.js etc). |
I have not updated yet from the build I previously stated. The developer console should be very telling in where the problem is when you are on a page that is currently suspended. I am out of town right now so I can't look further into the issue, but I will be glad to help next week when I am back at home if the issue is still being bothersome. |
Thx but this latest build has multiple refactoring changes and that is only one of the errors I am seeing but if I can put my finger on the issues I'll post back here. |
Nope TGS is now totally broken throwing this error upon any suspend operation as I indicated in open issue #823 that has been open for several commits now without any acknowledgement from @deanoemcke hopefully he is working on it:
|
It appears this is still an issue but with all the refactoring and with numerous other cascading errors that appeared unrelated (but were not) was a PITA to track down but I finally have and it tests good even with latest DEV/Canary builds. I'm creating a PR for it now hopefully Dean can get it merged soon. |
Cross origin security blocking prevents TGS from initializing when it cannot complete favIcon operations. This requires Anonymous declaration of crossOrigin. More info: https://stackoverflow.com/questions/49503171/the-image-tag-with-crossorigin-anonymous-cant-load-success-from-s3 https://bugs.chromium.org/p/chromium/issues/detail?id=409090#c23 https://bugs.chromium.org/p/chromium/issues/detail?id=718352#c10
Well I guess based on comment 23 here we shouldn't feel bad for this being such a PITA: https://bugs.chromium.org/p/chromium/issues/detail?id=409090#c23 I also referenced a few other links in the PR including that this very https://bugs.chromium.org/p/chromium/issues/detail?id=718352#c10 so this workaround apparently specific to Chrome is unfortunately necessary. In this case, debugging the resulting cascading errors that due to the current refactoring allowed it to appear as though enough initialization has occurred for TGS to function is really what cost a lot of time - while the actual resulting fix that you also found needed to be relocated and then was not nearly as time consuming to implement. :) |
Fixed in #827 @paysonblackwell thanks for identifying the issue. and @CollinChaffin thanks for implementing :) |
I've noticed since the last major update that was pushed, that all the tab's favicons would be a missing file icon instead, at least for me. After getting a little bit annoyed with that I decided to mess around and find the error.
Looks like it is in the suspended.js file, in the getFaviconMetaData() function.
This following line of code had the error: The canvas has been tainted by cross-origin data.
var imageData = context.getImageData(0, 0, canvas.width, canvas.height);
Looking online looks like there is a small fix to add this line of code right after the img variable is initialized.
img.crossOrigin = "Anonymous";
(I added it to Line No. 422)I added it to my local version and it fixed the problem and there isn't anymore errors showing up in the Developer Console.
I thought I should create an issue so this can be solved if others are having similar problems.
The text was updated successfully, but these errors were encountered: