1321712 - Clicking outside of the download panel should dismiss the panel and sub-menu

Status: RESOLVED DUPLICATE of bug 1362207

(Firefox :: Downloads Panel, defect, P5)

Description

[Ming-Chou Shih [:stone]]

This is the follow up of bug1301708 comment#21 case 3.

Comment 1

[Ming-Chou Shih [:stone]]

The new implementation of downloads is composed by download panel + menu. When clicking outside of the panel and menu, current implementation on Windows rolls up the menu. Users have to click again to roll up the panel. UX feedbacks [1] that we should roll up both the panel and sub-menu.

Traced the implementation and found that pass aCount=UINT32_MAX to nsXULPopupManager::Rollup will roll up all popups with the same type. Wondering if we should pass aCount=0 to nsXULPopupManager::Rollup to roll up all popups.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1301708#c21

Comment 2

[Ming-Chou Shih [:stone]]

Attachment: Bug 1321712 - Clicking outside of the download panel should dismiss the panel and sub-menu.

Review commit: https://reviewboard.mozilla.org/r/98266/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/98266/

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Hmm, sorry, I cannot decide if this is a good behavior. Could you check agreements of UX team or something before me?

I think that ideally, each popup should decide if it should be clicked at closing its child popup, though. For example, doesn't this change the behavior when you open dropdown on bookmark panel (at the star button) too? So, this could break some compatibility of our UI and UI of add-ons.

And can you create a mochitest-chrome for checking the behavior?

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8818154 [details]
Bug 1321712 - Clicking outside of the download panel should dismiss the panel and sub-menu.

So, please request review to me again after ui-review is granted.

Comment 5

[Ming-Chou Shih [:stone]]

Hi Morpheus,
About the behavior of clicking outside of the download panel commented in [1] case 3, may I have your comments that should this behavior only be applied to download panel or it should be applied to all popups?

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1301708#c21

Comment 6

[Morpheus Chen [:morpheus] UX]

I assumed the "all popups" you meant here are all door hangers triggered from the toolbar, for instance: bookmarks and Pocket, etc. If I understand correctly, I would say that the behavior(see https://bug1301708.bmoattachments.org/attachment.cgi?id=8806251) can be only applied to download panel since download panel is the only popup on the toolbar containing the feature of drop marker. In order to prevent any misunderstanding, please elaborate more if I make no sense here.

Comment 7

[Ben Tian (inactive)]

Paolo,

The new dropmarker (bug 1269962) hits comment 1 problem because download panel and dropmarker are of different types (panel vs menu). Do you have suggestion whether comment 2 change that rolls up all popups ensures UI compatibility mentioned in comment 3? If not, do you think revising dropmarker from menu to panel is feasible in terms of additional effort required?

Let me know if you have other suggestion.

Comment 8

[Ben Tian (inactive)]

Paolo, any thoughts on comment 7?

Comment 9

[:Paolo Amadini]

Hi Ben, sorry for the delay on this issue. I learned something new about the rollup mechanisms by reading these bugs and the related code, but I need to sit down for some time to understand the possible solutions and the effort involved. I have some reviews to close before I can take a closer look here.

Comment 10

[:Paolo Amadini]

Sorry again for the delay here.

In my opinion, the previous commenters are right in being conservative about changing the behavior of popup rollup across the entire user interface.

One of the examples made is when you click the star button on the toolbar to open the "Page Bookmarked" panel, then open the "Folder" dropdown to make a choice. At present, you can click anywhere in the browser window to close the dropdown, but the panel remains open to allow other edits. With this change, if you wanted to close the dropdown but keep the panel open, you would have to click inside the panel.

Technically, the proposed new behavior is as valid as the current one, and it doesn't introduce totally new problems because clicking outside of the browser window already closes both popups at once.

However, I can see how making this change would likely surprise some categories of users, as they would need to change their habits related to how they interact with the user interface. Considering this and the very minimal gain, I don't think it's worth changing for all panels and menus.

With regard to implementing this rollup behavior for the Downloads Panel only, we cannot just change the popup type of the submenu, because there is functional code that depends on it. For example, menu item commands are updated only if the popup is a menu:

https://dxr.mozilla.org/mozilla-central/rev/aa3e49299a3aa5cb0db570532e3df9e75d30c2d1/layout/xul/nsXULPopupManager.cpp#952-953

There may be similar code that depends on the popup type for resizing and positioning, so I don't think we can change the type of the panel to look like a menu.

We can of course make changes to the rollup code so that individual <menupopup> or <panel> elements can specify whether they want to be coalesced with the parent with regard to rollup behavior. This would require a new attribute or a new popupBoxObject method, and the patch should include regression tests. You may have different opinions, but personally I'm not sure it's worth the effort.

We can still experiment with workarounds in pure front-end code. Maybe we can close the Downloads Panel manually when the submenu is hidden, if the focus is not inside the panel. I'm not sure this would work though, because we actually consume the rollup event, so the focus might be in the panel even if the user clicked outside. The mouse pointer position is also not a reliable indication because you can also close the submenu with the keyboard.

Even for front-end workarounds, we still need regression tests and we have to make sure that they work across all platforms.

All in all, the effort of changing the default behavior just for the Downloads Panel might be disproportionate to the benefit. For what it's worth, I would be totally fine with shipping the current rollup behavior. Let me know what you'd like to do, I'm available for reviews if needed.

Comment 11

[Wesly Huang (EPM)]

Per check with QA this issue would be a blocker for dropmarker, which is in our backlog. So put this one in the backlog as well.

Comment 12

[Ben Tian (inactive)]

Clear ni? per comment 11.