Closed
Bug 1349187
Opened 7 years ago
Closed 7 years ago
Firefox crashes when popups close, if 'disable popup auto hide' is enabled
Categories
(Toolkit :: General, defect)
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox52 | --- | unaffected |
firefox-esr52 | --- | unaffected |
firefox53 | --- | wontfix |
firefox54 | --- | fixed |
firefox55 | --- | fixed |
People
(Reporter: jaws, Assigned: kats)
References
Details
(Keywords: regression)
Attachments
(1 file)
59 bytes,
text/x-review-board-request
|
enndeakin
:
review+
gchang
:
approval-mozilla-aurora+
lizzard
:
approval-mozilla-beta-
|
Details |
STR: Open the Web Console (Ctrl+Shift+K) Click the Settings button Enable "Enable browser chrome and add-on debugging toolboxes" Enable "Enable remote debugging" Open the Browser Toolbox (Ctrl+Alt+Shift+I) Enable the "Disable popup auto hide" button Open the following page: data:text/html,<select><option>1<option>2 Open the select popup Click on the webpgae Expected: The popup should close Actual: Firefox exits Regression range: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8dd496fd015a2b6e99573070279d9d1593836ea9&tochange=0cc9dced786cf2a3baeff707b636b4ad02637df0 Of that regression range, my best guess is Bug 1343977. Flagging needinfo for Kats and Neil.
Flags: needinfo?(enndeakin)
Flags: needinfo?(bugmail)
Comment 1•7 years ago
|
||
Looks like lastRollup isn't being initialized to null before calling Rollup in nsWindow::DealWithPopups. (The old code asserted when it wasn't null) Better would be to set it to null in the Rollup implementations, even when false is returned.
Flags: needinfo?(enndeakin)
Assignee | ||
Comment 2•7 years ago
|
||
Makes sense, thanks for looking into it! I can write the patch, unless you're already doing it.
Flags: needinfo?(bugmail)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → bugmail
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8849557 -
Flags: review?(enndeakin)
Assignee | ||
Updated•7 years ago
|
Has Regression Range: --- → yes
Has STR: --- → yes
status-firefox52:
--- → unaffected
status-firefox53:
--- → unaffected
status-firefox54:
--- → unaffected
status-firefox55:
--- → affected
Keywords: regression
OS: Unspecified → Windows
Hardware: Unspecified → All
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8849557 [details] Bug 1349187 - Ensure the Rollup implementations clear the out-pointer even upon returning false. https://reviewboard.mozilla.org/r/122334/#review124444
Attachment #8849557 -
Flags: review?(enndeakin) → review+
Pushed by kgupta@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5064bfb98923 Ensure the Rollup implementations clear the out-pointer even upon returning false. r=enndeakin+6102
Comment 6•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5064bfb98923
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Updated•7 years ago
|
status-firefox-esr52:
--- → unaffected
Assignee | ||
Comment 7•7 years ago
|
||
Comment on attachment 8849557 [details] Bug 1349187 - Ensure the Rollup implementations clear the out-pointer even upon returning false. Approval Request Comment [Feature/Bug causing the regression]: bug 1343977 [User impact if declined]: Crash when following the STR in comment 0 (and probably other STR as well) [Is this code covered by automated tests?]: no [Has the fix been verified in Nightly?]: no [Needs manual test from QE? If yes, steps to reproduce]: yes, STR in comment 0 [List of other uplifts needed for the feature/fix]: bug 1343977 [Is the change risky?]: no [Why is the change risky/not risky?]: small fix to ensure a variable is not left uninitialized. [String changes made/needed]: none
Attachment #8849557 -
Flags: approval-mozilla-beta?
Attachment #8849557 -
Flags: approval-mozilla-aurora?
Comment 8•7 years ago
|
||
Hi :kats, I have a few questions. 1. the bug number in [Feature/Bug causing the regression] is incorrect because you just fixed the bug in 55. So it's not a feature/regression in previous versions. 2, You also marked 53/54 unaffected. I'm not sure why you nominate the uplift request for 53/54.
Flags: needinfo?(bugmail)
Assignee | ||
Comment 9•7 years ago
|
||
I requested uplift to 53/54 on the regressing bug as well. This needs to be uplifted along with that. Right now this bug does not affect 53/54 so it would be incorrect to set the flags to affected... But once bug 1343977 is uplifted those branches will be affected.
Flags: needinfo?(bugmail)
Updated•7 years ago
|
Comment 10•7 years ago
|
||
I couldn't reproduce the initial issue on Nightly 55.0a1 2017-03-16 build on Win 10 64-bit machine, Microsoft surface Pro 2 tablet with Win 10 Insider Preview build and Mac OS X 10.12.4. Note that the option "Disable popup auto hide" is checked by default on Browser Toolbox - Settings - Available Toolbox Buttons section. Any thoughts? Thank you!
Comment 11•7 years ago
|
||
Comment on attachment 8849557 [details] Bug 1349187 - Ensure the Rollup implementations clear the out-pointer even upon returning false. This patch needs to be uplift along with bug 1343977. Aurora54+.
Attachment #8849557 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 12•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/0d45f6bfb7a6
Comment 13•7 years ago
|
||
Comment on attachment 8849557 [details] Bug 1349187 - Ensure the Rollup implementations clear the out-pointer even upon returning false. A little late in beta for taking this, but good that we fix it for 54.
Attachment #8849557 -
Flags: approval-mozilla-beta? → approval-mozilla-beta-
Updated•7 years ago
|
Reporter | ||
Comment 14•7 years ago
|
||
Hey Liz, I'd like to ask that beta-uplift be reconsidered here. The patch is very simple and fixes a reproducible crasher.
Flags: needinfo?(lhenry)
Assignee | ||
Comment 15•7 years ago
|
||
The crasher shouldn't be on beta, because the regressing bug (1343977) wasn't uplifted to beta.
Flags: needinfo?(lhenry)
You need to log in
before you can comment on or make changes to this bug.
Description
•