Popup blocked icon in url bar does not disappear even if the site is moved

VERIFIED FIXED in Firefox -esr52

Status

()

Firefox
General
--
minor
VERIFIED FIXED
a year ago
7 months ago

People

(Reporter: atlanto, Assigned: Gijs)

Tracking

({regression})

49 Branch
Firefox 55
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox50 wontfix, firefox51- wontfix, firefox52- wontfix, firefox-esr52 verified, firefox53+ verified, firefox54 verified, firefox55 verified)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

a year ago
str
STR:
1. Open any web page having popup to be blocked. (or local html file with <body onload="window.open('https://www.google.com/')">)
2. Make sure popup blocked icon appears in url bar.
3. Move to any other site.
ER:
The icon disappears.
AR:
The icon stays appearing.

mozregression:
 7:09.17 INFO: Last good revision: 8682f94befeb413bafc9e434d3cb387b1ce567d5
 7:09.17 INFO: First bad revision: 3a87296fe4145138c2ce15512bb31f76fe869cb4
 7:09.17 INFO: Pushlog:
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=8682f94befeb413bafc9e434d3cb387b1ce567d5&tochange=3a87296fe4145138c2ce15512bb31f76fe869cb4

 7:10.18 INFO: Looks like the following bug has the changes which introduced the regression:
https://bugzilla.mozilla.org/show_bug.cgi?id=1273685

Updated

a year ago
Blocks: 1273685
Status: UNCONFIRMED → NEW
status-firefox50: --- → affected
status-firefox51: --- → affected
status-firefox52: --- → affected
status-firefox53: --- → affected
tracking-firefox51: --- → ?
tracking-firefox52: --- → ?
tracking-firefox53: --- → ?
Ever confirmed: true
Flags: needinfo?(felipc)
Keywords: regression
Version: 50 Branch → 49 Branch
status-firefox50: affected → wontfix
Regression since Firefox 49 - I'm marking this fix-optional for 51 and 52 though if we get a simple fix, we could still uplift it.
status-firefox51: affected → fix-optional
status-firefox52: affected → fix-optional
tracking-firefox51: ? → -
tracking-firefox52: ? → -
tracking-firefox53: ? → +
Neil, can you help find someone to fix this regression? It sounds fairly annoying. Thanks.
Flags: needinfo?(enndeakin)

Comment 3

10 months ago
I don't know anything about this.
Flags: needinfo?(enndeakin)
(Assignee)

Updated

9 months ago
Duplicate of this bug: 1348387
(Assignee)

Comment 5

9 months ago
I'll see if I can find time tomorrow to have a look.
Flags: needinfo?(gijskruitbosch+bugs)
(Assignee)

Updated

9 months ago
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(felipc)
Comment hidden (mozreview-request)

Comment 7

9 months ago
mozreview-review
Comment on attachment 8849580 [details]
Bug 1325841 - fix hiding popup icon when navigating the browser,

https://reviewboard.mozilla.org/r/122362/#review125104

::: browser/base/content/browser.js:577
(Diff revision 1)
>  
>      if (!this._reportButton)
>        this._reportButton = document.getElementById("page-report-button");
>  
>      if (!gBrowser.selectedBrowser.blockedPopups ||
> -        gBrowser.selectedBrowser.blockedPopups.count == 0) {
> +        !gBrowser.selectedBrowser.blockedPopups.length) {

the beauty of dynamic languages..
Attachment #8849580 - Flags: review?(felipc) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 10

9 months ago
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/41284bd3ce5b
fix hiding popup icon when navigating the browser, r=Felipe

Comment 11

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/41284bd3ce5b
Status: ASSIGNED → RESOLVED
Last Resolved: 9 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
(Assignee)

Comment 12

9 months ago
Comment on attachment 8849580 [details]
Bug 1325841 - fix hiding popup icon when navigating the browser,

Approval Request Comment
[Feature/Bug causing the regression]: bug 1273685
[User impact if declined]: popup blocker icon hangs around after you go to a different page
[Is this code covered by automated tests?]: yes; this change adds some more.
[Has the fix been verified in Nightly?]: not yet
[Needs manual test from QE? If yes, steps to reproduce]: see comment #0
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: no
[Why is the change risky/not risky?]: I realize this looks big, but it's a 1-line change. We were checking for "foo.count", as it were, and we should have been checking for "foo.length". D'oh. Everything else is just tests (which I also happen to be moving).

Note that the patch will likely need a little help to uplift to beta/aurora  because I butchered browser/base/content/test/general/browser.ini in another bug. Happy to provide branch patches for that, or let relman deal with the (trivial) merge conflicts on that file.
[String changes made/needed]: none
Attachment #8849580 - Flags: approval-mozilla-beta?
Attachment #8849580 - Flags: approval-mozilla-aurora?

Updated

9 months ago
Component: XUL Widgets → General
Product: Toolkit → Firefox
Target Milestone: mozilla55 → ---

Updated

9 months ago
Target Milestone: --- → Firefox 55

Updated

9 months ago
status-firefox54: --- → affected
Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(brindusa.tot)

Updated

9 months ago
Flags: qe-verify+
Created attachment 8851575 [details]
pop-up blocker.mp4

Build ID: 20170326030204
User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:55.0) Gecko/20100101 Firefox/55.0

Verified on Firefox Nightly 55.0a1 on Windows 8.1 x 64, Mac OS X 10.12 and Ubuntu 14.04 x64. While testing the fix, I have observed that the pop-up blocked icon is displayed for a few seconds after the URL has been changed and until the new page has been loaded. If the user clicks on the icon, the pop-up blocker drop-down menu will be displayed. The drop-down menu is displayed even after the webpage has been loaded and the pop-up blocked icon has disappeared. 

Is this expected?

In my opinion if the pop-up blocked icon has disappeared when the webpage URL has been changed, there is not need to show again the icon until the page has finished loading.
Flags: needinfo?(felipc)
Flags: needinfo?(brindusa.tot)
(Assignee)

Comment 15

9 months ago
(In reply to Vlad Bacia-Mociran [:VladB] from comment #14)
> Created attachment 8851575 [details]
> pop-up blocker.mp4
> 
> Build ID: 20170326030204
> User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:55.0) Gecko/20100101
> Firefox/55.0
> 
> Verified on Firefox Nightly 55.0a1 on Windows 8.1 x 64, Mac OS X 10.12 and
> Ubuntu 14.04 x64. While testing the fix, I have observed that the pop-up
> blocked icon is displayed for a few seconds after the URL has been changed
> and until the new page has been loaded. If the user clicks on the icon, the
> pop-up blocker drop-down menu will be displayed. The drop-down menu is
> displayed even after the webpage has been loaded and the pop-up blocked icon
> has disappeared. 
> 
> Is this expected?
> 
> In my opinion if the pop-up blocked icon has disappeared when the webpage
> URL has been changed, there is not need to show again the icon until the
> page has finished loading.

Is the behaviour you describe from after this patch different from the behaviour before we fixed bug 1273685? Can you provide more detailed steps to reproduce this problem (ie what new site that loads so slowly are you testing with, etc.)?
Flags: needinfo?(felipc) → needinfo?(vlad.bacia)
Worth taking on ESR52 as well?
status-firefox51: fix-optional → wontfix
status-firefox52: fix-optional → wontfix
status-firefox-esr52: --- → affected
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to :Gijs from comment #15)
> Is the behaviour you describe from after this patch different from the
> behaviour before we fixed bug 1273685? Can you provide more detailed steps
> to reproduce this problem (ie what new site that loads so slowly are you
> testing with, etc.)?

The behaviour that I've described is after the patch was applied. In the video from comment 14, the popup blocker icon is clearly visible for a few seconds (or fractions of a second, depending on the internet connection speed) until youtube is loaded or any webpage. While the page loads, the popup blocker button can be clicked and the dropdown menu is displayed.

I've tested this issue on Nightly v53.0a1, Build ID: 20161201030205 (to see the difference) and in this build the popup blocker icon is still displayed after the page without popups has loaded. Build affected by this bug.

My questions are: 
The popup blocker icon should be displayed during the time in which the new page without popups is loaded? Is this expected? Video is in comment 14.
Flags: needinfo?(vlad.bacia)
(Assignee)

Comment 18

9 months ago
(In reply to Vlad Bacia-Mociran [:VladB] from comment #17)
> (In reply to :Gijs from comment #15)
> > Is the behaviour you describe from after this patch different from the
> > behaviour before we fixed bug 1273685? Can you provide more detailed steps
> > to reproduce this problem (ie what new site that loads so slowly are you
> > testing with, etc.)?
> 
> The behaviour that I've described is after the patch was applied. In the
> video from comment 14, the popup blocker icon is clearly visible for a few
> seconds (or fractions of a second, depending on the internet connection
> speed) until youtube is loaded or any webpage. While the page loads, the
> popup blocker button can be clicked and the dropdown menu is displayed.
> 
> I've tested this issue on Nightly v53.0a1, Build ID: 20161201030205 (to see
> the difference) and in this build the popup blocker icon is still displayed
> after the page without popups has loaded. Build affected by this bug.
> 
> My questions are: 
> The popup blocker icon should be displayed during the time in which the new
> page without popups is loaded? Is this expected? Video is in comment 14.

I understand your question, but I don't know what the answer is, which is why I've been asking you another question. I'm trying to understand if the state *after both this bug and bug 1273685* (which is the regressor for this bug) is different from *before both this bug and bug 1273685*. And you've not answered that question, because 20161201030205 is after bug 1273685, not before. Please can you find out if the behaviour you're describing is a regression compared to *before* bug 1273685.

If this happened before bug 1273685, this needs to be a new bug and we should find the regression window appropriately and treat it with the relevant level of importance, but it's unrelated to this bug. This is what I suspect.

If this didn't happen before bug 1273685, this either needs to be a new bug or a follow-up fix in this bug, because there might be something wrong with the patch for this bug.
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(vlad.bacia)
(Assignee)

Comment 19

9 months ago
(In reply to Ryan VanderMeulen [:RyanVM] from comment #16)
> Worth taking on ESR52 as well?

Only if the patch isn't broken, see comment #15 / comment 18.
(In reply to :Gijs from comment #18)
> Please can you find out if the behaviour you're describing is a
> regression compared to *before* bug 1273685.
> 
> If this happened before bug 1273685, this needs to be a new bug and we
> should find the regression window appropriately and treat it with the
> relevant level of importance, but it's unrelated to this bug. This is what I
> suspect.

I've tested again with Nightly v47.0a1, Build ID: 20160201030241 and the behaviour described by me is the same with the one before bug 1273685. In conclusion, the issue from comment 14 is a "new one" and a new bug should be created. I will log the new bug with the corresponding details.

From my point of view, the patch is ok and the issue is fixed.
status-firefox55: fixed → verified
Flags: needinfo?(vlad.bacia)
(Assignee)

Comment 21

9 months ago
Comment on attachment 8849580 [details]
Bug 1325841 - fix hiding popup icon when navigating the browser,

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: noticeable bug in the UI, 1-line fix in the code (with added automated tests)
User impact if declined: annoying icon stays in the URL bar, possibly confusing users
Fix Landed on Version: 55
Risk to taking this patch (and alternatives if risky): low, has automated tests, small patch, verified by QE
String or UUID changes made by this patch: nope.

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8849580 - Flags: approval-mozilla-esr52?
(Assignee)

Comment 22

9 months ago
(In reply to Vlad Bacia-Mociran [:VladB] from comment #20)
> I've tested again with Nightly v47.0a1, Build ID: 20160201030241 and the
> behaviour described by me is the same with the one before bug 1273685. In
> conclusion, the issue from comment 14 is a "new one" and a new bug should be
> created. I will log the new bug with the corresponding details.
> 
> From my point of view, the patch is ok and the issue is fixed.

Thanks! Please feel free to needinfo me on the new bug you file.
Comment on attachment 8849580 [details]
Bug 1325841 - fix hiding popup icon when navigating the browser,

let's take this in aurora54
Attachment #8849580 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
seems this need a rebasing for aurora
Flags: needinfo?(gijskruitbosch+bugs)
(Assignee)

Comment 25

9 months ago
remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/39cc9278d2fab01442a3349169f0f86805a152e9
status-firefox54: affected → fixed
Flags: needinfo?(gijskruitbosch+bugs)
same for beta, needs rebasing
(Assignee)

Comment 27

9 months ago
(In reply to Sylvestre Ledru [:sylvestre] from comment #26)
> same for beta, needs rebasing

It doesn't have beta approval, it seems, but I'm 99% sure that the aurora patch should graft cleanly onto beta.
Comment on attachment 8849580 [details]
Bug 1325841 - fix hiding popup icon when navigating the browser,

Let's try this on beta 9. It is late for this though. Is it such a common behavior or situation for users that we'd want it on ESR?
Attachment #8849580 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 29

9 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/bfd15258dba8
status-firefox53: affected → fixed
Flags: in-testsuite+
(Assignee)

Comment 30

9 months ago
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #28)
> Is it such a common
> behavior or situation for users that we'd want it on ESR?

I don't know. Depends how often people use sites that have blocked popups, and then if they navigate on those sites and then notice the icon remaining. I've barely ever seen it myself, but I'm hardly a typical user. :-)

It doesn't look like we have data on this.

TBH, the actual fix is basically a 1-liner, so from that perspective I'm pretty comfortable taking it in ESR. Happy to put up a patch if you like.
Flags: needinfo?(lhenry)
(Assignee)

Comment 31

9 months ago
Vlad, did you end up filing a separate bug for the pre-existing issue you identified?
Flags: needinfo?(vlad.bacia)
Yes(In reply to :Gijs from comment #31)
> Vlad, did you end up filing a separate bug for the pre-existing issue you
> identified?

Yes. Bug 1352966 has been created. Thank you.
Flags: needinfo?(vlad.bacia)
Comment on attachment 8849580 [details]
Bug 1325841 - fix hiding popup icon when navigating the browser,

let's take this in esr52
Flags: needinfo?(lhenry)
Attachment #8849580 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+

Comment 34

9 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-esr52/rev/74e9f13c554f
status-firefox-esr52: affected → fixed

Comment 35

9 months ago
I have reproduced this issue using Firefox 52.0a2 (20161225004004) on Win 8.1 x64.
I can confirm this issue is fixed, I verified using Firefox 53.0b9, 54.0a2 on Win 8.1 x64, Mac OS X 10.11 and Ubuntu 16.04 x64.
status-firefox53: fixed → verified
status-firefox54: fixed → verified
Flags: qe-verify+
Timea, could you please verify this on 52.1esr as well?
Flags: needinfo?(timea.zsoldos)

Comment 37

8 months ago
I can confirm this issue is fixed, I verified using Firefox 52.1esr on Win 8.1 x64, Mac OS X 10.11 and Ubuntu 14.04 x64.
Status: RESOLVED → VERIFIED
status-firefox-esr52: fixed → verified

Updated

8 months ago
Flags: needinfo?(timea.zsoldos)

Updated

7 months ago
Depends on: 1370018
You need to log in before you can comment on or make changes to this bug.