963078 - [Australis] Subviews (such as Bookmarks, history, devtools, character encoding, ...) run below the task bar when they are in the panel

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Attachment: Defect

When the bookmarks toolbar item is placed in the panel menu, the list of bookmarks can extend all the way down and below the task bar. This makes some items unreachable.
This happens on Windows 8.1 (so I'm assuming it is also an issue on Windows 7). The problem doesn't occur on XP (the panel doesn't expand there).

Comment 1

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

This also happens with the history, which makes the issue slightly worse because history is in the panel menu by default.

Comment 3

[Charles Evans]

History menu covers taskbar on Linux !Platform->+?

Mozilla/5.0 (X11; Linux i686; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140121063910 CSet: 1b52aa569ced

Comment 4

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Attachment: Defect on XP

Confirming that it also happens on Linux.
On XP, the panel does get a scroll bar, but it still runs below the task bar making »Show all History« unreachable.
On Windows 8 and Linux, there isn't a scroll bar at all. OS X (10.9) seems to work fine.

Comment 5

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Upgrading the priority.
This effectively makes it impossible to access the history library without either customizing Firefox first or making a round-trip via the downloads or bookmarks panel.
Also it affects all Windows versions, so it will hit lots of users.

Comment 7

[:Gijs (he/him)]

I'm guessing we just need to teach the relevant multipanelview height setter code to Not Do That. What surprises me is that I don't think this used to happen until recently. Does anyone know what changed?

Secondly... how do we know "where to stop", as it were? Did the panel code use to do this and does it no longer, or something?

Comment 9

[:Gijs (he/him)]

Investigating this. If you move the window down, the main panel also goes underneath the taskbar on Win7.

Comment 10

[:Gijs (he/him)]

So, the reason we go underneath rather than over the top of the task bar is bug 936187. Matt, can we back that part out? It doesn't seem to make any difference to the anchoring of the menu panel itself for me. Can you elaborate on why you removed it?

Meanwhile, let me see if we can figure out a way to not extend it down quite so far.

Comment 11

[Matthew N. [:MattN]]

(In reply to :Gijs Kruitbosch from comment #10)
> So, the reason we go underneath rather than over the top of the task bar is
> bug 936187. Matt, can we back that part out? It doesn't seem to make any
> difference to the anchoring of the menu panel itself for me. Can you
> elaborate on why you removed it?

level="top" shouldn't be used in this case as it makes a panel which isn't anchored to the window and the panel would appear above other applications when they are in the foreground. Of course you don't notice most of these issues since the panel normally hides when you switch applications or go to drag the window.

With the UITour we want to keep the panel open in most cases while the user is exploring and clicking the navigation arrows in the webpage content.
When level="top" is on the menu panel then there are three issues:
A) switching applications leave the menu panel floating above other applications
B) moving the window doesn't move the menu panel with it. The menu panel will stay floating in its initial position.
C) Tour annotations (highlights and info panels) appear behind the menu panel. The level="top" would need to propagate to all annotations and then they now also get bugs A and B with them (even while highlighting items in the toolbar for example, unless we special-case highlighting the panel). I also seem to recall that setting level="top" on the annotations still didn't allow them to go above the menu panel but I'm not sure if that is something I could have fixed.

All three were reported by others working on the tour as things to be fixed. You can see the tour at https://www.mozilla.org/en-US/firefox/29.0a2/whatsnew/

> Meanwhile, let me see if we can figure out a way to not extend it down quite
> so far.

A proper fix IMO would be for the panels to handle this so they don't go above or below the taskbar. That seems to be how menus work on Windows normally. I have seen code in Gecko that knows the height of the taskbar and this would be useful for our panels to support regardless. Otherwise, we can do like IE's bookmark/history/feed view and limit the height of the panel to the height of the browser window.

If we can't do one of these proper fixes above, we can probably do some UITour workarounds but remember that by-definition level="top" is not right for this use:
i) UITour could remove level="top" when it sets noautohide and restore it afterwards. This bug about running below the taskbar would occur but only during the UITour.
ii) Don't use noautohide during the tour and instead constantly re-open the menu panel when it closes. I don't think this is an acceptable UX as it would happen every time the user clicked to go to the next step of the tour.
iii) Remove noautohide for the tour, leave level="top" on the menu panel and perhaps we could use an event listener to cancel hiding the menu panel in certain cases (e.g. clicks in-content) but still close when the user clicks to moves the window chrome or when they switch applications.

We should check with Holly and Zhenshuo before implementing one of these workarounds as they are doing UX for the tour.

Comment 12

[:Gijs (he/him)]

(In reply to Matthew N. [:MattN] from comment #11)
> > Meanwhile, let me see if we can figure out a way to not extend it down quite
> > so far.
> 
> A proper fix IMO would be for the panels to handle this so they don't go
> above or below the taskbar. That seems to be how menus work on Windows
> normally. I have seen code in Gecko that knows the height of the taskbar and
> this would be useful for our panels to support regardless.

It turns out there is code that is meant to take care of this:

http://mxr.mozilla.org/mozilla-central/source/layout/xul/nsMenuPopupFrame.cpp#112

This looks up a LookAndFeel constant:

http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsLookAndFeel.cpp#360

Which is hardcoded to 'yes' on Windows.

It seems to me this is wrong for popups that go underneath rather than on top of the taskbar. I'm not sure under what conditions that happens, ie whether it is wholly controlled by level="top" or also by other factors. Neil?

Comment 13

[[e-mail]]

Unfortunately all the native popups that I can think of are level="top", so I'm not sure whether that's a sufficient condition for overlapping the taskbar. It is a necessary condition, of course, so it would be a good starting point.

Comment 16

[:Gijs (he/him)]

Fixed by bug 627974. I'll request uplift there.

Comment 18

[Cornel Ionce [:noni] [Hubs QA]]

The subviews are no longer displayed under the taskbar on latest Aurora (build ID: 20140406004002) and on Firefox 29 beta 5 (build ID: 20140403132807).
Marking issue as verified.