Closed Bug 1015617 Opened 11 years ago Closed 10 years ago

Intermittent browser_962069_drag_to_overflow_chevron.js | Uncaught exception - Panel did not hide within 20 seconds. | Uncaught exception - Panel did not show within 20 seconds.

Categories

(Firefox :: Toolbars and Customization, defect)

Product:
Firefox
Component:
Toolbars and Customization
Type:
defect
Priority:
Not set
Severity:
normal
Points:
3

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 39
Iteration:
39.1 - 9 Mar
Tracking Flags:
Tracking Status
firefox37 --- fixed
firefox38 --- fixed
firefox39 --- fixed
firefox-esr24 --- unaffected
firefox-esr31 --- unaffected

People

(Reporter: philor, Assigned: enndeakin)

References

Details

(Keywords: intermittent-failure)

Attachments

(4 files)

Patch v1: close the panel directly, instead of relying on keyboard events and window focus
1.51 KB, patch
Gijs
: review+
mikedeboer
: checkin+
Details | Diff | Splinter Review
listen for hiding the panel immediately
2.12 KB, patch
mikedeboer
: review+
Gijs
: checkin+
Details | Diff | Splinter Review
wait for popupshown
2.93 KB, patch
Gijs
: review+
Details | Diff | Splinter Review
Patch as checked in
2.07 KB, patch
lsblakk
: approval-mozilla-aurora+
lsblakk
: approval-mozilla-beta+
Details | Diff | Splinter Review
https://tbpl.mozilla.org/php/getParsedLog.php?id=40329492&tree=Mozilla-Inbound Windows 7 32-bit mozilla-inbound debug test mochitest-browser-chrome-1 on 2014-05-23 22:01:14 PDT for push d6c58a2a6da4 slave: t-w732-ix-025 https://tbpl.mozilla.org/php/getParsedLog.php?id=40328602&tree=Mozilla-Inbound Rev5 MacOSX Mountain Lion 10.8 mozilla-inbound debug test mochitest-browser-chrome-1 on 2014-05-23 21:49:39 PDT for push d29af2ac48cf slave: talos-mtnlion-r5-073 22:19:33 INFO - TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_962069_drag_to_overflow_chevron.js | Overflow panel is shown. 22:19:33 INFO - --DOCSHELL 32222D20 == 52 [pid = 3464] [id = 1540] (more GC, since nothing else is happening) 22:19:53 WARNING - TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_962069_drag_to_overflow_chevron.js | Uncaught exception - Panel did not hide within 20 seconds.
Assignee: nobody → jaws
Jared, do you think you'll be able to work on this soonish? If not, I can take a look!
Flags: needinfo?(jaws)
It's all yours :)
Assignee: jaws → mdeboer
Status: NEW → ASSIGNED
Flags: needinfo?(jaws)
Marco, can you add this to the current iteration?
Flags: needinfo?(mmucci)
Flags: firefox-backlog+
Whiteboard: p=3 [qa-]
Added to Iteration 32.3
Flags: needinfo?(mmucci)
Whiteboard: p=3 [qa-] → p=3 s=it-32c-31a-30b.3 [qa-]
Comment on attachment 8434152 [details] [diff] [review] Patch v1: close the panel directly, instead of relying on keyboard events and window focus Review of attachment 8434152 [details] [diff] [review]: ----------------------------------------------------------------- rs=me ::: browser/components/customizableui/test/browser_962069_drag_to_overflow_chevron.js @@ +23,5 @@ > let overflowChevron = document.getElementById("nav-bar-overflow-button"); > ChromeUtils.synthesizeDrop(identityBox, overflowChevron, [], null); > yield panelShownPromise; > > ok(true, "Overflow panel is shown."); Unrelated, but this should actually test something, like: is(widgetOverflowPanel.state, "open", ...) but that is fragile because the panel might have been hidden again by the time the (async!) .then() of the promise causes this code to continue and the test function to be called... ... or use info() instead. But no ok(true), please. :-)
Attachment #8434152 - Flags: review?(jaws) → review+
Thanks, Gijs! I also adjusted the line you noticed. https://hg.mozilla.org/integration/fx-team/rev/c9db5a1356bc
Attachment #8434152 - Flags: checkin+
https://hg.mozilla.org/mozilla-central/rev/c9db5a1356bc
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32
Status: RESOLVED → VERIFIED
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Sooo... looks like we're not done yet. Is it plausible that the panel gets hidden before the listener is added?
IOW, something like this?
Attachment #8434993 - Flags: review?(mdeboer)
Comment on attachment 8434993 [details] [diff] [review] listen for hiding the panel immediately Review of attachment 8434993 [details] [diff] [review]: ----------------------------------------------------------------- Let's try it! Can you add newlines before and after change? Just a nit :)
Attachment #8434993 - Flags: review?(mdeboer) → review+
*the change
Comment on attachment 8434993 [details] [diff] [review] listen for hiding the panel immediately remote: https://hg.mozilla.org/integration/fx-team/rev/707b7bcc4fca
Attachment #8434993 - Flags: checkin+
Keywords: leave-open
(In reply to TBPL Robot from comment #105) > philor > https://tbpl.mozilla.org/php/getParsedLog.php?id=41171422&tree=Mozilla- > Central > Ubuntu VM 12.04 x64 mozilla-central debug test mochitest-browser-chrome-1 on > 2014-06-05 19:46:55 > revision: c08a299f2163 > slave: tst-linux64-spot-622 > > TEST-UNEXPECTED-FAIL | > chrome://mochitests/content/browser/browser/components/customizableui/test/ > browser_962069_drag_to_overflow_chevron.js | Uncaught exception - Panel did > not hide within 20 seconds. Just to save everyone else the detective work I just did, this was just before the last patch hit m-c, and the try push (comment #106) also didn't have it.
Ditto for the try stars. Considering previous rate of failure, I am hopeful that this is it, but let's wait until the end of the weekend.
Sounds like this is FIXED?
Looks like it!
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Keywords: leave-open
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
(In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #113) > Sounds like this is FIXED? (In reply to :Gijs Kruitbosch from comment #114) > Looks like it! I see with my ears, it's confusing.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Added to Iteration 33.1
Whiteboard: p=3 s=it-32c-31a-30b.3 [qa-] → p=3 s=33.1 [qa-]
These new failures look more like something caused by bug 994117.
Status: REOPENED → ASSIGNED
Iteration: --- → 33.2
Points: --- → 3
QA Whiteboard: [qa-]
Whiteboard: p=3 s=33.1 [qa-]
Iteration: 33.2 → 33.3
Iteration: 33.3 → 34.1
Removed from Iteration 34.1 based on review by GMC.
Assignee: mdeboer → nobody
Status: ASSIGNED → NEW
Iteration: 34.1 → ---
Summary: Intermittent browser_962069_drag_to_overflow_chevron.js | Uncaught exception - Panel did not hide within 20 seconds. → Intermittent browser_962069_drag_to_overflow_chevron.js | Uncaught exception - Panel did not hide within 20 seconds. | Uncaught exception - Panel did not show within 20 seconds.
What's happening here is that we: - Synthesize the drop on the overflow button - Open the popup and wait for the popupshown promise to resolve - The popup opens and performs the opening transition - There's a timeout in CustomizableUI.jsm::_showWithTimeout that hides the popup 500ms after it has shown if it isn't being hovered. - In a slower debug build, this can happen before the opening transition has finished. This failure can easily be reproduced by changing OVERFLOW_PANEL_HIDE_DELAY_MS to a small number. It seems like _showWithTimeout should not be adding the hide timeout until the popupshown has happened.
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Attachment #8571712 - Flags: review?(gijskruitbosch+bugs)
Iteration: --- → 39.1 - 9 Mar
QA Whiteboard: [qa-]
Flags: qe-verify-
Comment on attachment 8571712 [details] [diff] [review] wait for popupshown Review of attachment 8571712 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/customizableui/CustomizableUI.jsm @@ +4199,5 @@ > > _hideTimeoutId: null, > _showWithTimeout: function() { > + Task.spawn(function () { > + yield this.show(); TBH, I would probably not import Task.jsm here and just this.show().then(...) (make sure to use .bind(this)!), but either way works for me. @@ +4204,5 @@ > + > + let window = this._toolbar.ownerDocument.defaultView; > + if (this._hideTimeoutId) { > + window.clearTimeout(this._hideTimeoutId); > + this._hideTimeoutId = null; This seems useless because we replace the timeout immediately anyway.
Attachment #8571712 - Flags: review?(gijskruitbosch+bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/771403b7b486
Status: ASSIGNED → RESOLVED
Closed: 11 years ago → 10 years ago
status-firefox39: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: Firefox 32 → Firefox 39
Looks like this is sticking. Can you please nominate this for Aurora/Beta approval when you're comfortable doing so? :)
Flags: needinfo?(enndeakin)
Approval Request Comment Fixes frequent intermittent orange test [Feature/regressing bug #]: [User impact if declined]: none. The patch changes the code for the overflow button to not add the popup hiding event listener until the popup has shown. [Describe test coverage new/current, TreeHerder]: frequently failing before, but no failures since this patch [Risks and why]: none [String/UUID change made/needed]: none
Flags: needinfo?(enndeakin)
Attachment #8576009 - Flags: approval-mozilla-beta?
Attachment #8576009 - Flags: approval-mozilla-aurora?
Comment on attachment 8576009 [details] [diff] [review] Patch as checked in approving for both beta/aurora to help with the intermittent test failure.
Attachment #8576009 - Flags: approval-mozilla-beta?
Attachment #8576009 - Flags: approval-mozilla-beta+
Attachment #8576009 - Flags: approval-mozilla-aurora?
Attachment #8576009 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: