Closed Bug 948213 Opened 6 years ago Closed 6 years ago

Menu Panel stays open when clicking recently closed tabs items in the history subview

Categories

(Firefox :: Bookmarks & History, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 29

People

(Reporter: tawn, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

(Whiteboard: [Australis:P4])

Attachments

(2 files, 2 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:27.0) Gecko/20100101 Firefox/27.0 (Beta/Release)
Build ID: 20131204004002

Steps to reproduce:

1. Click Australis Menu (hamburger) button
2. Click History
3. Click one of the History links (above Restore all Tabs)
4. Note that the History menu stays open
5. Click another History link (above Restore all Tabs)


Actual results:

Step 3 works fine. Step 5 either has no effect or opens incorrect history entry.


Expected results:

The proper link should always be opened. Either the menu should close after step 3 or the menu needs to be live updated so history items are associated with the correct url.
Links in ,History seems to be working fine on Nightly 28.0a1 (2013-12-08)

Confirmed : "History drop down menu doesn't close after clicking on a link"

New bug noticed : The menu drop down is pushed upwards as soon as `History` tab is clicked.
(See attachment)
Summary: History Button inside Australis Menu Panel opens wrong urls → Menu Panel stays open when clicking recently closed tabs items in the history subview
(In reply to Sudheesh Singanamalla (:ShellHacker) from comment #2)
> New bug noticed : The menu drop down is pushed upwards as soon as `History`
> tab is clicked.
> (See attachment)

This is bug 941196.
See Also: → 947332
(In reply to Sudheesh Singanamalla (:ShellHacker) from comment #2)
> Links in ,History seems to be working fine on Nightly 28.0a1 (2013-12-08)

I am still seeing the 'wrong url opened' bug on 29.0a1 (2013-12-10). Can you reproduce that issue with these steps?

Create new profile
Open https://www.mozilla.org/en-US/contribute/ ('Get Involved' from 'Recently Bookmarked')
Click new tab button
Open https://www.mozilla.org/en-US/firefox/customize/ ('Customize Firefox' from 'Recently Bookmarked')
Click new tab button
Close 'Get Involved' tab
Close 'Customize Firefox' tab
Click Australis Menu (PanelUI) button
Click History
Click 'Mozilla Firefox Web Browser-Customize' from the recently closed tabs area (it opens in a tab)
Note that the History menu stays open
Click 'Get Involved' from the recently closed tabs area (nothing happens; menu still open)
Click 'Customize' from the recently closed tabs area ('Get Involved' opens)

Regardless, fixing the "Menu Panel stays open..." (as the title of this bug was changed to) issue, should solve the problem.
Confirming, setting to NEW  I see the same behavior on win7 x64 with the latest hourly m-c win32 build Nightly
Status: UNCONFIRMED → NEW
Ever confirmed: true
Duplicate of this bug: 948687
Coming from bug 948687, I'd like the list to be live and not close specially for the "recently closed tabs" section as in my use case I cherry pick from that list after a tab closing rampage.

If the menu closed that would meant 3x clicks for my use case.

This should tie into bug 928843 too, right?
Blocks: 928843
Summary: Menu Panel stays open when clicking recently closed tabs items in the history subview → Recently closed tabs items in the history subview don't get removed when accessed, causing subsequent clicks to restore the wrong tabs
Whiteboard: [Australis:P4]
Component: Untriaged → Bookmarks & History
OS: Windows 7 → All
Hardware: x86 → All
Sorry for the summary change. In fact we should be closing the menu panel here to match the same behavior as the Menu Bar's History Recently-Closed-Tabs list.
Summary: Recently closed tabs items in the history subview don't get removed when accessed, causing subsequent clicks to restore the wrong tabs → Menu Panel stays open when clicking recently closed tabs items in the history subview
Duplicate of this bug: 964428
Judging by the new duplicate, seems to me like the consensus is that the list should update itself instead of close itself.

Could this be reconsidered, please?
Yes, I think it would be very useful to update the list instead of closing it.

Because of functionality: if someone would like to close it after restoring a tab, only a click outside the menu will close it. But if the list is closed automatically and someone wants to restore some tabs, he will get mad.

And because of coherence: the Developer list isn't closed neither.
(In reply to obrufau from comment #11)
> Yes, I think it would be very useful to update the list instead of closing
> it.
> 
> Because of functionality: if someone would like to close it after restoring
> a tab, only a click outside the menu will close it. But if the list is
> closed automatically and someone wants to restore some tabs, he will get mad.

This is only helpful if you want to restore more than one (but not all) tab(s). I posit that that is not the most common case.

> And because of coherence: the Developer list isn't closed neither.

That's a bug - but I can't reproduce it at all. Can you file a separate bug? Which developer item are you clicking that doesn't close the menu?
Flags: needinfo?(obrufau)
I don't think that's a bug neither :)

By the way, clicking the following commands doesn't close the Developer list: Toggle Tools, Web Console, Inspector, Debugger, Style Editor, Profiler, Network, Developer Toolbar, App Manager, Responsive Design View, Get More Tools, Work Offline.

Only these close it: Browser Console, Scratchpad, Page Source. And it makes sense, because they open a new window, so the old one loses focus and then the menu is closed.

Note: If the developer tools (Toggle Tools, Web Console, Inspector, Debugger, Style Editor, Profiler, Network) were not docked in the window, they will open in a new window. Therefore, the Developer menu will close too.

But I think that this current behavior is very coherent and intuitive, and it shouldn't change. Only the malfunctions (like History not updating) should be fixed.
Flags: needinfo?(obrufau)
(In reply to obrufau from comment #13)
> I don't think that's a bug neither :)
> 
> By the way, clicking the following commands doesn't close the Developer
> list: Toggle Tools, Web Console, Inspector, Debugger, Style Editor,
> Profiler, Network, Developer Toolbar, App Manager, Responsive Design View,
> Get More Tools, Work Offline.

I tried 5 of these, and all of them close the panel for me (yes, even when the devtools are docked). Have you tried on a clean profile? It *is* a bug if the panel isn't closing. You activated a menu item, it'll close the menu. It doesn't make sense that the menu would let you toggle between different developer tools after opening the first - you could just do that in the toolbox. Similarly, if the menu stayed open for e.g. the bookmarks view, that'd be weird, because if you click a bookmark, presumably that's the one you want, and we open it, and at that point the menu is just in the way of things. In this respect, it's no different from traditional menus.
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
You're right, with a clean profile the panel closes when clicking a developer item.
Comment on attachment 8367957 [details] [diff] [review]
fix recently closed tabs not closing panel in Australis panel subview,

Review of attachment 8367957 [details] [diff] [review]:
-----------------------------------------------------------------

I had been looking at this and didn't realize that the fix would be so subtle. nice.
Attachment #8367957 - Flags: review?(mconley) → review+
remote:   https://hg.mozilla.org/integration/fx-team/rev/2bff84532177
Whiteboard: [Australis:P4] → [Australis:P4][fixed-in-fx-team]
So this made the history view consistent with the developer and help subviews. That's fine.

Thinking about it more, however...

addPanelCloseListeners is called for the temporary panels we create in PanelUI.js (in case the buttons are in the toolbar), the overflow panel, and the main menu panel. However, even though the panel close listeners are on the main panel, they don't seem to get events for the subviews... this is going to bite everybody making subviews (see also: some of the last comments in bug 964944). Jared, when you were looking at this, did you figure out why that handler isn't dealing with stuff for us here?
Flags: needinfo?(jaws)
No I didn't figure out why.
Flags: needinfo?(jaws)
https://hg.mozilla.org/mozilla-central/rev/2bff84532177
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P4][fixed-in-fx-team] → [Australis:P4]
Target Milestone: --- → Firefox 29
This appears to be not fixed. 

Tested on win7 x64 m-c win32 build.
Tested on Tinderbox hourly build cset:
https://hg.mozilla.org/mozilla-central/rev/940a7d00c096
1. click on Menu/Hamburger 
2. click on History Icon
3. Pick any link in 'Recent closed tabs' list
4. Left or mid-click on link, link opens but the History Panel does not close, still being displayed. 

Am I missing something here in reference to what this bug was about ?
(In reply to Jim Jeffery not reading bug-mail 1/2/11 from comment #22)
> This appears to be not fixed. 
> 
> Tested on win7 x64 m-c win32 build.
> Tested on Tinderbox hourly build cset:
> https://hg.mozilla.org/mozilla-central/rev/940a7d00c096
> 1. click on Menu/Hamburger 
> 2. click on History Icon
> 3. Pick any link in 'Recent closed tabs' list
> 4. Left or mid-click on link, link opens but the History Panel does not
> close, still being displayed. 
> 
> Am I missing something here in reference to what this bug was about ?

On a clean profile? Because it works for me...
Flags: needinfo?(jmjeffery)
@ Gijs, 

Yes, just tested on a fresh Profile with no history what-so-ever, addons, etc...

History Panel will not close when clicking a 'recent closed tab' history item.
Flags: needinfo?(jmjeffery)
I should wear a dunce cap for like a week for committing what I did. Sigh.

'win' doesn't exist in that context. Why this worked in my testing, no idea. Either way, this probably also causes bug 966125.
Blocks: 966125
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to :Gijs Kruitbosch from comment #25)
> I should wear a dunce cap for like a week for committing what I did. Sigh.
> 
> 'win' doesn't exist in that context. Why this worked in my testing, no idea.
> Either way, this probably also causes bug 966125.

Backed out:

remote:   https://hg.mozilla.org/integration/fx-team/rev/60bff07e1074
Attachment #8367957 - Attachment is obsolete: true
Attachment #8367957 - Flags: review-
No longer blocks: 966125
How about this? We really shouldn't leave subviews with an inferior closing mechanism. Add-on authors have been running into this as well.
Attachment #8368723 - Flags: review?(jaws)
Comment on attachment 8368723 [details] [diff] [review]
Australis menu panel stays open when opening items in subviews,

Review of attachment 8368723 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good.
Attachment #8368723 - Flags: review?(jaws) → review+
Blocks: 964944
Mike, can you doublecheck this change from a customize mode transition perspective? I'm moving the ensureReady check because otherwise the close handler stuff can't find the panel anymore (because we move the panel into the panelholder). This is only important if we open customize mode without opening the panel first, otherwise this shouldn't really have any effect, I think?
Attachment #8368753 - Flags: review?(mconley)
Attachment #8368723 - Attachment is obsolete: true
Comment on attachment 8368753 [details] [diff] [review]
Australis menu panel stays open when opening items in subviews,

Review of attachment 8368753 [details] [diff] [review]:
-----------------------------------------------------------------

I'd say that while this might add a little bit of jank at the beginning of the transition, it's jank that would have showed up at the *end* of the transition before during the switch from customize-entering to customize-entered, so I guess it's an even trade.

I'd also say that this case (customize mode without opening the panel) is likely a rare one, so this trade is likely fine.
Attachment #8368753 - Flags: review?(mconley) → review+
Alright, hope it works better the second time around. :-)

remote:   https://hg.mozilla.org/integration/fx-team/rev/7ea80c5d5a1e
Status: REOPENED → ASSIGNED
Whiteboard: [Australis:P4] → [Australis:P4][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/7ea80c5d5a1e
Status: ASSIGNED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P4][fixed-in-fx-team] → [Australis:P4]
Depends on: 966684
QA Contact: cornel.ionce
Issue verified fixed on the latest Aurora (Build ID: 20140310004003) and latest Nightly (Build ID: 20140310030201), using:
- Windows 7 64-bit [1],
- Mac OS X 10.9.1 [2],
- Ubuntu 13.10 64-bit [3].

Note: each of the restored tabs is opened as the 1st tab (before the currently opened tab), I'm not sure if this is the intended behavior, please do confirm.

1. Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0
2. Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:29.0) Gecko/20100101 Firefox/29.0
3. Mozilla/5.0 (X11; Linux x86_64; rv:29.0) Gecko/20100101 Firefox/29.0
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.