Closed Bug 956704 Opened 7 years ago Closed 7 years ago
"Acid Defender" Web
Audio game audio doesn't continue playing after canceling the multiple tabs warning
239 bytes, text/html
15.57 KB, patch
|Details | Diff | Splinter Review|
2.38 KB, patch
|Details | Diff | Splinter Review|
STR: 1. Open http://www.cappel-nord.de/webaudio/acid-defender/ and enter "Jam Mode" 2. Open a new empty tab 3. Switch back to the game tab 4. Press the X button to close Firefox 5. Cancel the multiple tabs warning Actual results: The game is stopped Expected results: The game should continue playing Reproducible on 28.0a2 (2014-01-06), 29.0a1 (2014-01-05). Works fine on FF 27b2.
Last good nightly: 2013-12-17 First bad nightly: 2013-12-18 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b1e5ade62913&tochange=862cb6a1cc88
(In reply to Paul Silaghi, QA [:pauly] from comment #1) > Last good nightly: 2013-12-17 > First bad nightly: 2013-12-18 > > Pushlog: > http://hg.mozilla.org/mozilla-central/ > pushloghtml?fromchange=b1e5ade62913&tochange=862cb6a1cc88 Thanks Paul, could you try regressing this further with the tinderbox builds for this range?
Last good revision: cbe04dbc2bd6 First bad revision: 08dc60299942 Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=cbe04dbc2bd6&tochange=08dc60299942
Taking for investigation.
Assignee: nobody → paul
This works just fine on last tip. Feel free to reopen if you can repro.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WORKSFORME
Still repro for me on 29.0a1 (2014-01-14), Win 7 x64.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
So, I've found the issue, and the attached testcase show the difference in behaviour before and after mrbkap patch. The testcase adds listeners on the "blur" and "focus" events. STR: 1. Have a Firefox release (that does not have mrbkap's patch), and a Nightly build (that has it) at hand 2. Open Firefox release, ensuring that the option that says that Firefox should warn you if you have multiple tab on window closing (in setting, tab) 3. Open a new window 4. Open a new tab 5. Load the testcase in this new tab 6. Click on the arrow in order to trigger the confirmation window, notice that this sends a "blur" event to the document. 7. Cancel the window closing, notice that a "focus" event is sent to the document Now, repeat step 2 - 7 in the Nightly build, that has the patch. Notice that when the confirmation window appears, the "blur" event is not send to the document. Notice that when the cancel button is clicked, both events are sent to the document. This is the source of the change in behaviour in the demo, as the demo code has an elaborate scheduling logic based on "blur" and "focus".
Blake, is that something your patch could cause?
Hi Blake, Per Comment 8, this appears to be something your patch caused. So I'm reassigning this bug to you. If you think we're wrong, please let us know. Thanks.
Assignee: paul → mrbkap
I looked into this today. I have a patch that should fix this, but partially regresses bug 933483. I'll have it finished tomorrow.
This should be the safest patch for this bug. Unfortunately, when I try this patch on the testcase in bug 933483, I sometimes fail after dismissing my modal alert. I haven't found STR for it yet, and my debugging has been hampered by lldb's sad interaction with the JITs. I'll look again next week, but I wanted to get the patch into review in the meantime.
Attachment #8376428 - Flags: review?(bugs)
Comment on attachment 8376428 [details] [diff] [review] Only suspend animation frames This doesn't have the same setup for animation pausing as what we have for event handling suppressing. Things get wrong if some new document is loaded while animations are paused etc. See http://mxr.mozilla.org/mozilla-central/search?string=SuppressEventHandling&find=\.cpp&findi=\.[chj]&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
Attachment #8376428 - Flags: review?(bugs) → review-
Comment on attachment 8378520 [details] [diff] [review] Introduce a mechanism for only suppressing animation frames. Docshell part isn't super pretty, but should be rare case.
Attachment #8378520 - Flags: review?(bugs) → review+
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Paul, please verify this is fixed in tomorrow's Nightly.
Verified fixed FF 30.0a1(2014-02-28) Win 7, Ubuntu 13.04 and Mac OS X 10.8.5.
What's the risk/reward here of uplifting? We're building our final FF28 beta today - please let us know asap if this is a low enough risk fix to take on Beta.
Comment on attachment 8378520 [details] [diff] [review] Introduce a mechanism for only suppressing animation frames. We should definitely try to get this in on beta. [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 933483 User impact if declined: Various bugs dealing with alerts, confirmations and other UI elements, affecting web pages as well. Testing completed (on m-c, etc.): This has been on m-c for about a week with no known regressions. Risk to taking this patch (and alternatives if risky): This patch should be less risky than the patch for bug 933483, as it effectively is a less risky version of that patch. String or IDL/UUID changes made by this patch: None.
Comment on attachment 8378522 [details] [diff] [review] Only suppress animation frames when entering a modal state. Need this patch to actually fix the bug.
Reproduced with Nightly from 2014-01-20 by using STR from comment 0. Verified as fixed with Firefox 28 beta 9 (Build ID: 20140306171728): Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 Mozilla/5.0 (X11; Linux x86_64; rv:28.0) Gecko/20100101 Firefox/28.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:28.0) Gecko/20100101 Firefox/28.0
Verified as fixed using latest Aurora 29.0a2 (20140314004001) under Win 7 64-bit, Ubuntu 32-bit and Mac OSX 10.8.5.
You need to log in before you can comment on or make changes to this bug.