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.

Sign up for GitHub

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

Jump to bottom

Chrome: Opening a Twitter tab in background for 10 seconds causes script to fail #198

Closed
mcpower opened this issue Jan 21, 2023 · 5 comments · May be fixed by #199
Closed

Chrome: Opening a Twitter tab in background for 10 seconds causes script to fail #198

mcpower opened this issue Jan 21, 2023 · 5 comments · May be fixed by #199
Labels
bug Something isn't working

Comments

@mcpower
Copy link

mcpower commented Jan 21, 2023 •
edited
Loading

This issue occurs with both the Chrome extension (version 2.22.0) and the UserScript (version 88). Tested on a clean Chrome profile.

With Chrome 109, opening a Twitter page in the background, and waiting a few seconds before switching to the tab, causes Tweak New Twitter to not be injected. The culprit seems to be these lines:

https://github.com/insin/tweak-new-twitter/blob/62625cdfb562d3978847393637355288f7cd12c3/tweak-new-twitter.user.js#L3289-L3296

I suspect that Tweak New Twitter is being run, but the actual DOM isn't rendered until the user switches to the tab. Interestingly, Chrome renders the tab without a <title>, just the URL:

image

and it seems like hovering over the tab to activate Chrome's tab preview causes the DOM and <title> to be rendered.
mcpower added a commit to mcpower/tweak-new-twitter that referenced this issue Jan 21, 2023
@mcpower
Delay getElement timeout until after rAF
e8bcfc4 
Under normal circumstances, this should be a negligible increase in the
timeout, but if this tab is in the background, this rAF will delay until
it has focus.

Fixes insin#198.
@mcpower mcpower mentioned this issue Jan 21, 2023
Open
@insin insin added the bug Something isn't working label Jan 21, 2023
@insin
Copy link
Owner

insin commented Jan 21, 2023

Would just removing the timeout altogether fix this?

If Twitter ever change their implementation so the one specific element you currently need to look at to determine whether it's in mobile or desktop mode is no longer there, 99.9% of users aren't going to notice the logged message anyway, so the result is still just "Tweak New Twitter broke".

@insin
Copy link
Owner

insin commented Jan 21, 2023

Looks like just removing the timeout will work - it seems to get the app wrapper immediately if you're not logged in and waits for <title> to appear (which is what drives all the functionality based on the current page), or if you're logged in, it waits for the app wrapper instead then gets the <title>.

@mcpower
Copy link
Author

mcpower commented Jan 21, 2023

Yep, removing the timeout altogether would fix it too.

You bring up a good point re: "what if #layers + div doesn't work in the future". Removing the timeout would result in the rAF loop being run forever like a spinlock, and would presumably increase CPU and battery usage (although this may just be premature optimisation). Perhaps we could keep the timeout, and gracefully handle the null element case instead? Some ideas might be:

  • A crude window.alert() telling users that something failed.
  • Same as above, but as an unintrusive popup on the screen.
  • Assume that most users are using Desktop, and hope for the best.

@insin insin closed this as completed in f531d13 Jan 21, 2023
insin added a commit that referenced this issue Jan 21, 2023
@insin
Release v2.22.1
65027f0 
- Fixed Tweak New Twitter not working if you open Twitter in a background tab then switch to it some time later [#198]
@insin
Copy link
Owner

insin commented Jan 21, 2023

Thanks for the bug report - the current plan for that is still "I see it when I check Twitter and fix it as quickly as possible" 😸

Put this in as a quick fix but will swing back around to this after I'm finished getting the Safari versions ready.

@mcpower
Copy link
Author

mcpower commented Jan 21, 2023

I have a fix at #199 which adds a rAF to the timeout, which also fixes this issue if you wanted to use a timeout in the future. Feel free to close that if it's not necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Delay getElement timeout until after rAF mcpower/tweak-new-twitter
2 participants
@insin @mcpower