Windows and OS X only, since we lack support for translucent windows on Linux. I guess this can be seen as a subset of bug 610545...

Comment on attachment 635449 [details] [diff] [review] patch Review of attachment 635449 [details] [diff] [review]: ----------------------------------------------------------------- Looks really great! Just one odd thing I noticed is that on Windows pressing the button that spawned the panel with the panel still open reopens it instead of dismissing it. This happens with the Bookmark Panel and the Downloads Panel but not the Identity Panel. Seems to work fine on OS X though.

Comment on attachment 635803 [details] [diff] [review] patch v2 >+ <handler event="popupshown" phase="target"> >+ this.setAttribute("panelopen", "true"); >+ </handler> Would it be clearer to set the attribute on panel-arrowcontainer instead and simplify the css rules a bit, though it would require getting that element here? Either way is ok. >+ <handler event="popuphidden" phase="target"><![CDATA[ >+ this.removeAttribute("panelopen"); >+ >+ // Destroying the widget to prevent the current state from bein rendered >+ // briefly when the panel reopens. 'being'

(In reply to Neil Deakin from comment #3) > >+ <handler event="popupshown" phase="target"> > >+ this.setAttribute("panelopen", "true"); > >+ </handler> > > Would it be clearer to set the attribute on panel-arrowcontainer instead and > simplify the css rules a bit, though it would require getting that element > here? Either way is ok. I can use xbl:inherits.

http://hg.mozilla.org/integration/mozilla-inbound/rev/70e3a2c8c6b4 assuming ui-r=shorlander based on comment 1

Comment on attachment 635803 [details] [diff] [review] patch v2 Review of attachment 635803 [details] [diff] [review]: ----------------------------------------------------------------- (In reply to Dão Gottwald [:dao] from comment #5) > http://hg.mozilla.org/integration/mozilla-inbound/rev/70e3a2c8c6b4 > > assuming ui-r=shorlander based on comment 1 Yes! Looks great, thank you.

http://mozillamemes.tumblr.com/post/19498220636/try-server-takes-the-beatings-so-mozilla-inbound Backed out due to mochitest-other orange. https://hg.mozilla.org/integration/mozilla-inbound/rev/4ad52472d98f https://tbpl.mozilla.org/php/getParsedLog.php?id=12914676&tree=Mozilla-Inbound 13619 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_arrowpanel.xul | Test timed out.

This just adds the -moz-transform transition, the rest has been moved to separate bugs.

Some failures on OS X with this minimal patch: > browser/base/content/test/browser_bug553455.js | Should have reconstructed > the notification UI - Didn't expect [object XULElement], but got it > browser/base/content/test/browser_popupNotification.js | Test timed out > dom/indexedDB/test/browser_permissionsPromptDeny.js | Test timed out > dom/indexedDB/test/browser_privateBrowsing.js | Test timed out > dom/indexedDB/test/browser_quotaPromptDeny.js | Test timed out And Windows: > toolkit/content/tests/chrome/test_arrowpanel.xul | panel label is below > toolkit/content/tests/chrome/test_arrowpanel.xul | panel label is above > toolkit/content/tests/chrome/test_arrowpanel.xul | panel label is below > toolkit/content/tests/chrome/test_arrowpanel.xul | panel label is above [...] > browser/base/content/test/browser_bug553455.js | Should have reconstructed > the notification UI - Didn't expect [object XULElement], but got it

For whatever reason, if I reduce the transition distance to 7 pixels, the browser_bug553455.js and test_arrowpanel.xul failures go away. Enn, any idea what's going on there? I still get intermittent timeouts in browser_popupNotification.js and browser_quotaPromptDelete.js, specifically in triggerSecondaryCommand. The problem is that the button popup closes when the panel transition finishes. I don't think we need to worry about this in practice, but I'm not sure how to deal with it in the tests.

Probably the arrow isn't appearing correctly. The test_arrowpanel test mostly tests that the arrows appear on the right side when the panel is flipped due to space constraints, so it is very picky about small changes being made to the sizes of things. Can you identify which parts of the test are failing (by commenting out calls to openPopup/yield? I'm not familiar with the other test.

test_arrowpanel.xul fixed such that it doesn't care about transforms applied to the panel content. Blair, any idea why these supposedly harmless theme bits cause browser_bug553455.js to fail with "Should have reconstructed the notification UI"?

Comment on attachment 641386 [details] [diff] [review] patch v4 Review of attachment 641386 [details] [diff] [review]: ----------------------------------------------------------------- (In reply to Dão Gottwald [:dao] from comment #12) > Blair, any idea why these supposedly harmless theme bits cause > browser_bug553455.js to fail with "Should have reconstructed the > notification UI"? No idea, sorry. But I don't have much experience with arrow panels, or that test - Dave wrote all that.

Comment on attachment 641386 [details] [diff] [review] patch v4 This code should be clicking on the panel's anchor twice immediately after the panel opens (in the popupshown event). It looks like with these changes the anchor isn't seeing those clicks any more. I think the arrow panel is over the top of it at that point with this change. Waiting till the transform is done probably won't help much as the panel this test checks is short-lived and goes away when a very short download completes. Maybe just dispatching the click event directly to the anchor rather than using synthesizeMouse?

(In reply to Dave Townsend (:Mossop) from comment #14) > Comment on attachment 641386 [details] [diff] [review] > patch v4 > > This code should be clicking on the panel's anchor twice immediately after > the panel opens (in the popupshown event). It looks like with these changes > the anchor isn't seeing those clicks any more. I think the arrow panel is > over the top of it at that point with this change. The transform affects the panel content, not the panel itself; the panel remains at the same position throughout the transition and the content is cut off at the top rather than overflowing the panel. So if synthesizeMouse thinks the anchor is blocked, that's a bug :(

This was all green on the try server. Dave, can you review the test fixes / workarounds?

Comment on attachment 642319 [details] [diff] [review] patch v5 Review of attachment 642319 [details] [diff] [review]: ----------------------------------------------------------------- I think that's fine for now. Do you want to file a bug on the bad behaviour though.

(In reply to Dave Townsend (:Mossop) from comment #17) > Comment on attachment 642319 [details] [diff] [review] > patch v5 > > Review of attachment 642319 [details] [diff] [review]: > ----------------------------------------------------------------- > > I think that's fine for now. Do you want to file a bug on the bad behaviour > though. Filed bug 775089.

Comment on attachment 642319 [details] [diff] [review] patch v5 [Approval Request Comment] The first part of this already landed for Firefox 16 in bug 767861. This isn't a bad state to be in, but it would be nice to have the full animation in place as originally intended and ui-reviewed in this bug. Medium risk -- some tests didn't like the slide-in transition and had to be updated; however, the problems that these tests had aren't problems that would affect real users.

Comment on attachment 642319 [details] [diff] [review] patch v5 [Triage Comment] Still very early in the 16 cycle. We can take this on Aurora still at this point. If there's anything in particular QA can do to help us gain confidence here, please add instructions and the qawanted keyword. Thanks!