Closed
Bug 860994
Opened 11 years ago
Closed 11 years ago
Story - Fixup intermittent context menu failing tests
Categories
(Firefox for Metro Graveyard :: Tests, defect, P2)
Tracking
(Not tracked)
VERIFIED
FIXED
Firefox 23
People
(Reporter: rsilveira, Assigned: rsilveira)
References
Details
(Whiteboard: feature=story c=testing u=developer p=1)
Attachments
(1 file)
7.44 KB,
patch
|
mbrubeck
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•11 years ago
|
||
Occasionally context menu popup hide would not trigger a transitionend event and would end up in a zombie state where it's there but with opacity 0. After that, trying to pop up or hide a new context menu would fail. I was able to reproduce it very frequently by reducing the transition time to 0.02s. I'm still not 100% sure why this was happening, my feeling is that having a separate transition for opacity was confusing the engine somehow - may be worth investigating further. After adding the hiding attribute to trigger the hiding transition, it never happened again (on my machine™).
Attachment #736576 -
Flags: review?(mbrubeck)
Attachment #736576 -
Flags: review?(jmathies)
Updated•11 years ago
|
QA Contact: jbecerra
Whiteboard: p=1 → feature=story c=testing u=developer p=1
Updated•11 years ago
|
Summary: Fixup intermittent context menu failing tests → Story - Fixup intermittent context menu failing tests
Updated•11 years ago
|
Comment 2•11 years ago
|
||
Comment on attachment 736576 [details] [diff] [review] Patch v1 Review of attachment 736576 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/metro/base/content/helperui/MenuUI.js @@ -380,2 @@ > self._panel.hidden = true; > - self._popupState = null; Why is this line removed? Is _popupState nulled out elsewhere, or do we want it to remain set? ::: browser/metro/base/tests/mochitest/browser_context_menu_tests.js @@ -34,5 @@ > "Top position is " + aElement.top + ", expected between " + aMinTop + " and " + aMaxTop); > } > > gTests.push({ > - desc: "text context menu", I don't think this was a typo -- this is testing the context menus for selected *text*
Attachment #736576 -
Flags: review?(mbrubeck) → review+
Assignee | ||
Comment 3•11 years ago
|
||
(In reply to Matt Brubeck (:mbrubeck) from comment #2) > Comment on attachment 736576 [details] [diff] [review] > Patch v1 > > Review of attachment 736576 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: browser/metro/base/content/helperui/MenuUI.js > @@ -380,2 @@ > > self._panel.hidden = true; > > - self._popupState = null; > > Why is this line removed? Is _popupState nulled out elsewhere, or do we > want it to remain set? > I removed it because it was never used. This was the only reference. > ::: browser/metro/base/tests/mochitest/browser_context_menu_tests.js > @@ -34,5 @@ > > "Top position is " + aElement.top + ", expected between " + aMinTop + " and " + aMaxTop); > > } > > > > gTests.push({ > > - desc: "text context menu", > > I don't think this was a typo -- this is testing the context menus for > selected *text* Duh! Reverted it :)
Assignee | ||
Comment 4•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/37fd7284be3a
Comment 5•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/37fd7284be3a
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 23
![]() |
||
Updated•11 years ago
|
Attachment #736576 -
Flags: review?(jmathies)
Comment 7•11 years ago
|
||
For Raymond to test and verify.
Flags: needinfo?(kamiljoz) → needinfo?(mozbugs.retornam)
Comment 8•11 years ago
|
||
verified fixed http://hg.mozilla.org/mozilla-central/rev/fef5f202b2dc
Status: RESOLVED → VERIFIED
Flags: needinfo?(mozbugs.retornam)
Comment 9•11 years ago
|
||
No need to verify this manually anymore; if the test starts breaking we'll find out about it when the test breaks.
Updated•10 years ago
|
OS: Windows 8 Metro → Windows 8.1
You need to log in
before you can comment on or make changes to this bug.
Description
•