Skip to content
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

Fix for #821,822,823 #827

Merged
merged 1 commit into from
Dec 19, 2018
Merged

Conversation

CollinChaffin
Copy link
Contributor

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

TODO: Consider (continued) refactoring of favIcon (and other critical component) error handling/logging particularily during TGS startup/init

@Jollfye
Copy link
Contributor

Jollfye commented Dec 16, 2018

Tested your commit, it appears to be working on the latest Chrome 73.0.3639.1 (Official Build) dev (64-bit), TGS initiates with no errors (and so I'm not getting bombarded with that "Disable developer mode extensions" Chrome security message on every restart), all tabs with correct paled favicons after load, good. Tab suspending works, only just noticed one error when suspended new google.com tab for the first time, when donation notification arrived at the bottom left corner it was with no urled images, just text (see screeny), and this "new" error I saw in extension errors after:

scr1

scr1

Also Session management still looks like this as described in my issue #822 and throws the same error with '+' click:

scr2

scr3

So the help of @deanoemcke is still needed here.

@CollinChaffin
Copy link
Contributor Author

CollinChaffin commented Dec 18, 2018

That error is a totally different issue that I'll let @deanoemcke address I'm surprised to see new commits and not even a comment on this PR or the several open issues it fixes. I'm not tagged an official contributor so I could not commit direct or would have so hopefully Dean will at least acknowledge the contribution and tell me if it's okay or what exactly the issue is preventing commit.

EDIT: Understand for over 10 days NO commit in master has even fully initialized enough to suspend tabs on at least half the systems I test it on - so I jumped after 10 days to apply a tested fix that gets TGS up and running I've been running my fork with this commit 24/7 since without a single noticeable issue (other than the totally separate more cosmetic errors still needing addressing). You simply were not seeing these other errors before @Jollfye because TGS was SO broken it wouldn't init far enough to even get to these others. :)

@deanoemcke deanoemcke merged commit 0d186b5 into greatsuspender:master Dec 19, 2018
@deanoemcke
Copy link
Collaborator

@CollinChaffin thanks so much for this. Sorry, I've been away on holiday so only found out about this last night. Also, all my chrome 'dev' testing is done on Canary, so I'm a bit surprised that this only appears in the Dev channel. I assumed that Canary would also have these changes?

I will follow up this PR with some cleanup of the init error handling code as you suggested.

@Jollfye I'm investigating your two issues now and will push some fixes soon.

Thanks everyone for all the debugging. It's been invaluable.

@deanoemcke
Copy link
Collaborator

@Jollfye i have fixed the donate buttons issue.
for the session management issue, it appears that it's specific to your particular session. are you comfortable giving me an export of the session that's causing you issues?

you can always try updating and testing the latest version first if you'd prefer to see if it's magically been fixed first..

@Jollfye
Copy link
Contributor

Jollfye commented Dec 20, 2018

@deanoemcke I updated TGS to the latest 158 build, donate buttons confirm fixed, about session management, sent u an email.

@deanoemcke deanoemcke added this to the Chrome webstore 7.1 release milestone Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants