Closed Bug 872163 Opened 7 years ago Closed 7 years ago
Defect - Metro Firefox Settings, About, and Sync flyouts should close when Win8 Settings panel opens
1. Go to the settings charm, open the options panel (or about, or sync). 2. Go to the settings charm, open the permissions flyout You'll see 2 flyouts shown but only one should be shown. I notice permissions hides itself as soon as the settings charm is expanded, we should do this for the other flyouts too. p=2
Summary: Metro Firefox Settings, About, and Sync flyouts should close when Win8 Settings panel opens → Defect - Metro Firefox Settings, About, and Sync flyouts should close when Win8 Settings panel opens
Whiteboard: feature=defect c=tbd u=tbd p=0
:bbondy, is there anything this bug needs before it is [shovel-ready] ?
should be ready to go
Whiteboard: feature=defect c=tbd u=tbd p=0 → feature=defect c=tbd u=tbd p=0 [shovel-ready]
Assignee: nobody → ally
Whiteboard: feature=defect c=tbd u=tbd p=0 [shovel-ready] → feature=defect c=tbd u=tbd p=2
Hi Ally, if you'll be taking this in Iteration #8 please add the Bug ID of the stories the defect relates to as a block.
There is really a good one, so let's do the sync flyout & the about flyout since those appear to be the defective cases
Hides all xul flyouts if a xul flyout is selected. This means the windows 8 settings flyout still sits over the old one until another fly out is selected. This is a half fix best (you still see the overlay of hte setting panel but not hte 2 flyouts over each other) and at worst no better (there's still two things causing visual clutter). I think the answer lies elsewhere.
I commented the followup in the wrong bug >.< https://bugzilla.mozilla.org/show_bug.cgi?id=867495#c1 https://bugzilla.mozilla.org/show_bug.cgi?id=867495#c2 https://bugzilla.mozilla.org/show_bug.cgi?id=867495#c3
(In reply to Brian R. Bondy [:bbondy] from comment #3) > Actually I think a good place might be when the window is deactivated. If I > recall correctly, anytime a metro flyout is displayed our window will be > deactivated. > > http://dxr.mozilla.org/mozilla-central/widget/windows/winrt/FrameworkView. > cpp#l444 > > I think there's already an event sent there. There is an event, but I noticed halfway through today that "Content:Deactivate" doesn't fire when flyouts happen. Which is to say if that were true, than I expect to see http://mxr.mozilla.org/mozilla-central/source/browser/metro/base/content/bindings/browser.js#707 fire, and I do not. This is the event I would expect the Cpp to be sending as the function is called "SendActivationEvent()" :( Maybe it's actually sending a different message. Let's find out. > > This will also fix another bug of when you switch away from Metro the > permissions flyout dismisses but our flyouts don't.
I did some digging starting from SendActivationEvent(), called in FrameworkView::OnWindowActivated(). SendActivationEvent() http://dxr.mozilla.org/mozilla-central/widget/windows/winrt/FrameworkView.cpp#l322 of which the most relevant line is probably the call to http://dxr.mozilla.org/mozilla-central/widget/windows/winrt/MetroWidget.cpp#l1089 which tosses over to (a file that dxr could not find, but mxr can?) http://mxr.mozilla.org/mozilla-central/source/xpfe/appshell/src/nsWebShellWindow.cpp#397 whose key line I think is http://mxr.mozilla.org/mozilla-central/source/xpfe/appshell/src/nsWebShellWindow.cpp#405 which I think leads to this ball of wax http://mxr.mozilla.org/mozilla-central/source/dom/base/nsFocusManager.cpp#631 which to the best of my (very rusty) c++ does not raise any message, event, or observer notification. I think there is currently no way for the JS to be aware of this event, and that's the missing link. (pretty please bbondy, someone, tell me I'm wrong! :( ) I think starting at OnWindowActivated() might be the right path. So, I think this bug is going to need two parts: A)backend, cpp to message B) frontend, js to actually control our flyouts.
TimA suggests that http://mxr.mozilla.org/mozilla-central/source/dom/base/nsFocusManager.cpp#762 might actually be the answer.
looks like tim's suggestion is spot on. let's go with that
Comment on attachment 755118 [details] [diff] [review] FlyoutPanelsUI listening for "deactivate" event Review of attachment 755118 [details] [diff] [review]: ----------------------------------------------------------------- Yup seems like widget activated/deactivated leads to are activate/deactivate. There's a non exhaustive list of events here: https://developer.mozilla.org/en-US/docs/XUL/Events Looks good and this fixes a bunch of issues with the flyout being inconsistent with the permissions flyout.
Attachment #755118 - Flags: review?(netzen) → review+
I'm all for killing multiple bugs with one stone. https://hg.mozilla.org/integration/mozilla-inbound/rev/bd9e82e152d5
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24
For testing and verification.
verified for it8 - re-opening the setting charms menu using a side swipe while our flyout is visible closes our flyout.
Mozilla/5.0 (Windows NT 6.2; rv:24.0) Gecko/20130612 Firefox/24.0 Verified that all flyouts (Options, Sync, About, Permissions) are closed when Setting are open.
Status: RESOLVED → VERIFIED
User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0 Build ID: 20130816030205 Built from http://hg.mozilla.org/mozilla-central/rev/1ed5a88cd4d0 WFM Tested on windows 8 using latest nightly for iteration-12. Followed steps provided in comment0 and got expected result.
You need to log in before you can comment on or make changes to this bug.