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

Popup/popunder blocking broken after opening uBO's dashboard through popup panel #3057

Closed
gorhill opened this issue Sep 26, 2017 · 8 comments

Comments

@gorhill
Copy link
Owner

gorhill commented Sep 26, 2017

First reported here by @kasper93.

This is unrelated to the regression, which was fixed.

This affects all versions of Firefox, including in Nightly.

The repro steps are quite simple (from my end, both with Firefox 55 and Nightly):

  1. Launch browser.
  2. Open popup test page in a new tab.
  3. Verify that the popup from the first test ("direct url tab") is properly blocked.
    • uBO's popup blocking depends on webNavigation.onCreatedNavigationTarget to properly set the sourceTabId property.
  4. Open uBO's dashboard (or logger) through uBO's popup panel.
    • The issue can't be reproduced if opening the dashboard from uBO's preferences in about:addons.
  5. Go back to the popup test page previously opened above, and repeat the test in 3.

Result: uBO fails to properly filter the popup.

After much investigation, it turns out that the one call to vAPI.closePopup() in vapi-common.js is causing this issue:

window.open('','_self').close();

The issue does not exist with Chromium, with the same code path.

I have no idea why this happens. It appears Firefox's internals are in a bad state after step 4: the sourceTabId from webNavigation.onCreatedNavigationTarget is always -1.

@gorhill gorhill changed the title Popup/popunder blocking broken when opening uBO's dashboard through popup panel Popup/popunder blocking broken after opening uBO's dashboard through popup panel Sep 26, 2017
@kasper93
Copy link
Contributor

Hmm, I'm wondering why you do window.open('','_self').close();? window.close(); works just fine and doesn't break anything in the process. At least for Firefox.

@gorhill
Copy link
Owner Author

gorhill commented Sep 26, 2017

@kasper93 I don't know, I didn't put this code there: b301ac0#diff-bc664f26b9c453e0d43a9379e8135c6a.

It didn't occur to me to try window.close(), I assumed there must have been an issue with it as the rationale to use what is currently in there. I will give it a try, and if it works, I will use it just for Firefox until I understand why window.open('','_self').close() was used.

gorhill added a commit that referenced this issue Sep 26, 2017
@kasper93
Copy link
Contributor

@gorhill: Sure, I just read here https://developer.mozilla.org/en-US/Add-ons/WebExtensions/user_interface/Popups

The popup can be closed programmatically by calling window.close() from a script running in the popup.

So it seems to be blessed way to close popups. Might be hard to remember why you did this way after all this time. :)

@gorhill
Copy link
Owner Author

gorhill commented Sep 26, 2017

@kasper93, you're right, I confirm this works fine. Wish I had seen your comment before pushing rc2.

I removed my comment re "sticky" above to avoid any confusion.

gorhill added a commit to gorhill/uMatrix that referenced this issue Sep 26, 2017
@gorhill
Copy link
Owner Author

gorhill commented Sep 26, 2017

I removed the "browser bug" label, but I do wonder what is occurring internally for Firefox API's webNavigation.onCreatedNavigationTarget to become broken. I wonder if this issue can be triggered by a normal web page.

@gwarser
Copy link
Contributor

gwarser commented Sep 26, 2017

How can I close a browser window without receiving the “Do you want to close this window” prompt?

https://stackoverflow.com/questions/57854/how-can-i-close-a-browser-window-without-receiving-the-do-you-want-to-close-thi

@gorhill
Copy link
Owner Author

gorhill commented Sep 26, 2017

Putting back the "browser bug" label, this simple HTML document will cause webNavigation.onCreatedNavigationTarget to become broken:

<!DOCTYPE html>
<html>
<head></head>
<body>
<script>
    window.open('','_self').close();
</script>
</body>
</html>

@Rob--W
Copy link
Contributor

Rob--W commented Sep 26, 2017

Thanks for the debugging @gorhill and others. I have poked a bit more at it, created a test case and reported the bug at: https://bugzilla.mozilla.org/show_bug.cgi?id=1403349

Noxgrim pushed a commit to Noxgrim/uMatrix that referenced this issue Dec 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants