-
-
Notifications
You must be signed in to change notification settings - Fork 69
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Not handling popup blockers on initial load #228
Conversation
If pop-ups are blocked by your browser (Chrome), it can cause confusing exception `Uncaught TypeError: Cannot read property 'document' of null` followed by stack trace mentioning `mranderson`. The solution is to always allow pop-ups from your app under development URL (e.g. http://0.0.0.0:3449). This probably occurs only when the browser was closed previously with separate re-frame-10x debugging window. I have spent a long time to figure out, so I think it should be mentioned here to be easily searchable. Thanks!
Full stack trace:
|
Hey, I'd rather fix the error and log a good warning rather than just adding a note in the documentation here. |
Will be solved by writing info into js/console.
If pop-ups are blocked by Chrome, it can cause `Uncaught TypeError: Cannot read property 'document' of null`. This occurs only when the browser was closed previously with separate re-frame-10x debugging window. This fix writes an error message into console with info how to fix it.
Good idea, Daniel. So I committed a solution. |
@kajism I can see where the bug would be, but I'm having trouble reproducing it with Chrome on macOS. I've got Version 71.0.3578.98 (Official Build) (64-bit) on macOS 10.14.2. Even when I disable popups on the dev site (on localhost and 0.0.0.0), they still pop up. Can you give me some more instructions on how to reproduce this? In the meantime, I've pushed a fix for this. It shows an error message in the main window instead of logging to the console, as the user may not have their console open and miss the message. |
@danielcompton I'm using the same version of Chrome as you e.g. "Version 71.0.3578.98 (Official Build) (64-bit)" on Linux Mint (4.15.0-43-generic #46-Ubuntu SMP Thu Dec 6 14:45:28 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux). It can be reproduced this way:
Daniel, unfortunately with the current solution committed to master (9462c33), re-frame-10x window is not displayed again and the error stack trace in console is suppressed, so it fails silently. If I don't notice the little "Pop up were blocked on this page" displayed in Chrome, I'm lost completely until I try to press Ctrh-H to see the new error info you have provided. In addition the error message is displayed in re-frame-10x debugger even when the user don't care to open the separate pop up window. |
Great, thanks for the details, I can reproduce this now. The issue is only visible when re-frame-10x tries to automatically create a popup on initial page load. I'll fix it in this case too. |
If our JS tries to open a popup window without user interaction, Chrome will block this, as it is indistinguishable from a popup ad. To reproduce this issue: 1. Ensure the popup window is closed 2. In Local Storage, set "day8.re-frame-10x.external-window?" => "true" 3. Reload the browser 4. re-frame-10x will try and re-open the popup to restore the state If the popup window is already open, Chrome doesn't prevent you from 're-attaching' to it. Fixing this exposed a latent bug where we try to reopen an external window before we have dimensions loaded from localstorage, so this commit fixes that issue too. Fixes #228
I think I've cracked it this time, let me know if it's still an issue though. Thanks! |
The latest release works nicely. Congratulations and thanks for all your work, Daniel! |
If pop-ups are blocked by your browser (Chrome), it can cause confusing exception
Uncaught TypeError: Cannot read property 'document' of null
followed by stack trace mentioningmranderson
. The solution is to always allow pop-ups from your app under development URL (e.g. http://0.0.0.0:3449).This probably occurs only when the browser was closed previously with separate re-frame-10x debugging window. I have spent a long time to figure out, so I think it should be mentioned in README.md to be easily searchable.
Thanks!