Wrong pop-up message is displayed when running while(true) {window.open(...);}
Categories
(Toolkit :: UI Widgets, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox64 | --- | unaffected |
firefox65 | --- | wontfix |
firefox66 | --- | verified |
firefox67 | --- | verified |
People
(Reporter: mboldan, Assigned: Paolo)
References
Details
(Keywords: regression)
Attachments
(1 file)
47 bytes,
text/x-phabricator-request
|
lizzard
:
approval-mozilla-beta+
|
Details | Review |
Affected versions
- Firefox 65.0b9
- Firefox 66.0a1 (2019-01-09)
Affected platforms
- Windows 10x64
- Ubuntu 18.04x64
- macOS 10.12.6
Steps to reproduce
- Launch Firefox.
- Open Web Console.
- Run this script while (true) {window.open('http://google.fi');} and observe the displayed pop-up.
Expected result
- Nightly prevented this site from opening more than 100 pop-up windows. message is displayed.
Actual result
- Nightly prevented this site from opening a pop-up window message is displayed.
Regression range
- First bad: 65.0a1 (2018-11-14)
- Last good: 65.0a1 (2018-11-13)
- Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b9725b8b1c14bb944e5ed3526758fe966da79905&tochange=fcbdfdfb077626ac424844baed754b9f7168ada2
Additional notes
- On Firefox 64.0.2 RC it's displayed exactly the no. of pop-ups that are prevented for opening. It seems that this is the old behavior.
Comment 1•5 years ago
|
||
Bug 1496827 looks like a possible candidate there?
Comment 2•5 years ago
|
||
I don't understand why this assertion is not failing: https://searchfox.org/mozilla-central/rev/5053031ba7621fa8f63f42de4c204ab3561e4e59/browser/base/content/test/popups/browser_popup_blocker.js#36
Really strange.
Comment 3•5 years ago
|
||
Brian, flagging you for needinfo since you reviewed the offending patch. Could you take a look since Paolo hasn't gotten to it yet?
Comment 4•5 years ago
|
||
Too late for 65 at this point.
Assignee | ||
Comment 5•5 years ago
|
||
This fixes the notification bar that shows how many popup windows have been blocked.
Assignee | ||
Comment 6•5 years ago
|
||
This was lower priority, but it turns out it is also an easy fix.
Updated•5 years ago
|
Pushed by paolo.mozmail@amadzone.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/00074091644d Allow changing the text of an existing notification. r=jaws
Comment 8•5 years ago
|
||
Backed out changeset 00074091644d (bug 1519095) for browser-chrome failures at browser/base/content/test/popups/browser_popup_blocker.js
Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/cefe669c7351fc07d33f1c00979c81fdd7933d20
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=223521742&repo=mozilla-inbound&lineNumber=2757
08:12:21 INFO - TEST-START | browser/base/content/test/popups/browser_popupUI.js
08:12:22 INFO - GECKO(925) | Can't find symbol 'GetGraphicsResetStatus'.
08:12:22 INFO - GECKO(925) | MEMORY STAT vsizeMaxContiguous not supported in this build configuration.
08:12:22 INFO - GECKO(925) | MEMORY STAT | vsize 4157MB | residentFast 299MB | heapAllocated 129MB
08:12:22 INFO - TEST-OK | browser/base/content/test/popups/browser_popupUI.js | took 1152ms
08:12:23 INFO - checking window state
08:12:23 INFO - TEST-START | browser/base/content/test/popups/browser_popup_blocker.js
08:12:23 INFO - TEST-INFO | started process screencapture
08:12:24 INFO - TEST-INFO | screencapture: exit 0
08:12:24 INFO - Buffered messages logged at 08:12:23
08:12:24 INFO - Entering test bound setup
08:12:24 INFO - Leaving test bound setup
08:12:24 INFO - Entering test bound test_maximum_reported_blocks
08:12:24 INFO - Buffered messages finished
08:12:24 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/popups/browser_popup_blocker.js | Uncaught exception - at chrome://mochitests/content/browser/browser/base/content/test/popups/browser_popup_blocker.js:35 - TypeError: notification.label is undefined
08:12:24 INFO - Stack trace:
08:12:24 INFO - test_maximum_reported_blocks@chrome://mochitests/content/browser/browser/base/content/test/popups/browser_popup_blocker.js:35:3
08:12:24 INFO - AsyncTester_execTest/<@chrome://mochikit/content/browser-test.js:1108:34
08:12:24 INFO - asyncTester_execTest@chrome://mochikit/content/browser-test.js:1099:16
08:12:24 INFO - nextTest/<@chrome://mochikit/content/browser-test.js:997:9
08:12:24 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:803:59
08:12:24 INFO - Leaving test bound test_maximum_reported_blocks
08:12:24 INFO - Entering test bound test_opening_blocked_popups
08:12:24 INFO - TEST-PASS | browser/base/content/test/popups/browser_popup_blocker.js | Blocked popup menu shown -
08:12:24 INFO - TEST-PASS | browser/base/content/test/popups/browser_popup_blocker.js | Two popups were blocked -
08:12:24 INFO - Console message: [JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "http://example.com/browser/browser/base/content/test/popups/popup_blocker_b.html" line: 0}]
08:12:24 INFO - Console message: [JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "http://example.com/browser/browser/base/content/test/popups/popup_blocker_a.html" line: 0}]
08:12:24 INFO - TEST-PASS | browser/base/content/test/popups/browser_popup_blocker.js | Popup a -
08:12:24 INFO - TEST-PASS | browser/base/content/test/popups/browser_popup_blocker.js | Popup b -
08:12:24 INFO - Leaving test bound test_opening_blocked_popups
08:12:24 INFO - GECKO(925) | MEMORY STAT | vsize 4206MB | residentFast 333MB | heapAllocated 136MB
08:12:24 INFO - TEST-OK | browser/base/content/test/popups/browser_popup_blocker.js | took 1732ms
08:12:24 INFO - Not taking screenshot here: see the one that was previously logged
08:12:24 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/popups/browser_popup_blocker.js | Found an unexpected tab at the end of test run: http://example.com/browser/browser/base/content/test/popups/popup_blocker_10_popups.html -
08:12:24 INFO - checking window state
08:12:24 INFO - TEST-START | browser/base/content/test/popups/browser_popup_blocker_identity_block.js
08:12:25 INFO - GECKO(925) | ###!!! [Parent][DispatchAsyncMessage] Error: PQuotaUsageRequest::Msg_Cancel Route error: message sent to unknown actor ID
08:12:28 INFO - GECKO(925) | MEMORY STAT | vsize 4449MB | residentFast 344MB | heapAllocated 119MB
Updated•5 years ago
|
Updated•5 years ago
|
Pushed by paolo.mozmail@amadzone.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/f8aeed3352c0 Allow changing the text of an existing notification. r=jaws
Assignee | ||
Updated•5 years ago
|
Comment 10•5 years ago
|
||
bugherder |
Assignee | ||
Comment 11•5 years ago
|
||
Comment on attachment 9038040 [details]
Bug 1519095 - Allow changing the text of an existing notification. r=jaws
Beta/Release Uplift Approval Request
Feature/Bug causing the regression
User impact if declined
The notification bar for blocked pop-ups won't mention how many were blocked
Is this code covered by automated tests?
Yes
Has the fix been verified in Nightly?
No
Needs manual test from QE?
Yes
If yes, steps to reproduce
Per comment 0
List of other uplifts needed
None
Risk to taking this patch
Low
Why is the change risky/not risky? (and alternatives if risky)
Simple code change, the issue wasn't detected by tests but they've now been updated to check the actual label
String changes made/needed
None
Updated•5 years ago
|
Reporter | ||
Comment 12•5 years ago
|
||
I managed to reproduce this issue on Firefox 66.0b7, under Windows 10x64.
The issue is no longer reproducible on Firefox 67.0a1 (2019-02-13).
Tests were executed under Window 10x64, macOS 10.12.6 and under Ubuntu 16.04x64.
I will mark the issue Verified Fixed and remove the 'qe-verify +' flag as soon as it gets verified on a beta build.
Comment 13•5 years ago
|
||
Comment on attachment 9038040 [details]
Bug 1519095 - Allow changing the text of an existing notification. r=jaws
Let's uplift this for beta 8 and get further verification there as Mihai suggests.
Comment 14•5 years ago
|
||
bugherder uplift |
Reporter | ||
Comment 15•5 years ago
|
||
(In reply to Mihai Boldan, QA [:mboldan] from comment #12)
I managed to reproduce this issue on Firefox 66.0b7, under Windows 10x64.
The issue is no longer reproducible on Firefox 67.0a1 (2019-02-13).
Tests were executed under Window 10x64, macOS 10.12.6 and under Ubuntu
16.04x64.
I will mark the issue Verified Fixed and remove the 'qe-verify +' flag as
soon as it gets verified on a beta build.
I've also verified the issue on Firefox 66.0b8, under Windows 10x64, Mac OS X 10.11.6 and under Ubuntu 18.04x64.
Marking this issue as Verified Fixed.
Updated•5 years ago
|
Description
•