Closed Bug 1054840 Opened 5 years ago Closed 5 years ago
Unexpected popup blocker notification pops up in Firefox31, Aurora33
.0a2 and Nightly34 .0a1
[Tracking Requested - why for this release]: regression This problem happens on Firefox31, Aurora33.0a2 and Nightly34.0a1. This problem does not happen on Firefox30, 32.0b7, IE11 and Chrome36. Steps To Reproduce: 1. Open attached 2. Open any other page Actual Results: Unexpected popup blocker notification pops up Expected Results: Popup blocker notification should not pop up Regression window(m-i) Good: https://hg.mozilla.org/integration/mozilla-inbound/rev/dbed599ec413 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0 ID:20140320161729 Bad: https://hg.mozilla.org/integration/mozilla-inbound/rev/257d152e487f Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0 ID:20140320164730 Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=dbed599ec413&tochange=257d152e487f Regressed by: 8c129d201f96 Bill McCloskey — Bug 933462 - [e10s] Pop-up blocking notifications (r=felipe) Pregression window in (aurora32 only) Bad: https://hg.mozilla.org/releases/mozilla-aurora/rev/51243afbb1a9 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:32.0) Gecko/20100101 Firefox/32.0 ID:20140711202012 Fixed: https://hg.mozilla.org/releases/mozilla-aurora/rev/b3f6dbd37993 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:32.0) Gecko/20100101 Firefox/32.0 ID:20140711211411 Pushlog: http://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?fromchange=51243afbb1a9&tochange=b3f6dbd37993 Fixed by: b3f6dbd37993 Bill McCloskey — Bug 1024391 - Backout bug 933462 for breaking Android. a=lmandel
Bill, can you help here? Tracking because it is a regression and 32 is fixed (while 33 is not)
5 years ago
No longer depends on: 1047111
This is a separate problem. Here's what happens in without the popup code for e10s: 1. Web page asks to open a popup in onunload. 2. We show the popup blocker UI. 3. We get a pagehide event for the page, which nulls out all the popup info and removes the popup UI from the URL bar, but not from the notification box. 3. We hit the onLocationChange code here, which removes the notification box: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#4213 Here's what happens with the e10s version: 1. Same. 2. Content script sends a message asking to show popup blocker UI. 3. Content script sends another message after pagehide that nulls out the popup data. 4. We hit onLocationChange, but no notification is showing, so we do nothing. 5. We receive the first message and show the popup UI. 6. We get the second message and remove the UI from the URL bar, but not from the notification box. So the basic issue is that using messages causes stuff to happen at a different time relative to onLocationChange. This actually shouldn't be a problem in an e10s build because we use messages for everything, including onLocationChange. I'd like to fix this by removing the notification box when we get the second popup message. I don't see why we don't do this already. It's not really very useful once we've nulled out blockedPopups since we won't be able to show anything anyway.
Assignee: nobody → wmccloskey
Status: NEW → ASSIGNED
Attachment #8474882 - Flags: review?(felipc)
Per the dupe and my own experience, this issue also affects 32 beta. Are we sure it was definitely backed out and should be fixed now?
Comment on attachment 8474882 [details] [diff] [review] popup-notification-fix Review of attachment 8474882 [details] [diff] [review]: ----------------------------------------------------------------- With this, can't we remove the code that removed the notificationbox in OnLocationChange?
(In reply to :Felipe Gomes from comment #7) > Comment on attachment 8474882 [details] [diff] [review] > popup-notification-fix > > Review of attachment 8474882 [details] [diff] [review]: > ----------------------------------------------------------------- > > With this, can't we remove the code that removed the notificationbox in > OnLocationChange? I think that code is responsible for all notifications, not just the popup blocker ones. So I think we still need it. Also, this is not a dupe of bug 1055197 since it doesn't affect the URL bar icon--only the notification box.
Attachment #8474882 - Flags: review?(felipc) → review+
Comment on attachment 8474882 [details] [diff] [review] popup-notification-fix Approval Request Comment [Feature/regressing bug #]: bug 933462 [User impact if declined]: Weird popup blocking behavior if popup is shown when navigating away from site. [Describe test coverage new/current, TBPL]: On m-c. [Risks and why]: There is some risk of regressions, but not much. [String/UUID change made/needed]: None
Attachment #8474882 - Flags: approval-mozilla-aurora?
I don't think we will take this patch for esr.
Attachment #8474882 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Per comment 6, Gijs is able to repro this issue on 32. Does 32 require this fix, which looks safe enough? If so, we'll want to land this by tomorrow (Aug 21).
(In reply to Lawrence Mandel [:lmandel] from comment #13) > Per comment 6, Gijs is able to repro this issue on 32. Does 32 require this > fix, which looks safe enough? If so, we'll want to land this by tomorrow > (Aug 21). I'm sorry for the confusion. I was confused by the very similar-sounding issues. The issue I was experiencing is bug 1055197, which in turn is apparently a dupe of bug 1045826 instead of this one. See comment #8.
Using latest Aurora and Firefox 33 beta 2 (not e10s) the pop-up notification is displayed until the website loads and then it disappears. Using e10s new window I can`t see the pop-up so the issue is fixed but only on e10s. For the non e10s windows is this the intended behavior?
I think the current behavior is fine. This is a pretty weird corner case.
Thanks Bill, given comment 17 I`m marking this issues as verified fixed based on the testing from comment 16.
Depends on: 1374983
You need to log in before you can comment on or make changes to this bug.