310474 - Context menu might be shown below a richlistitem when invoked with the keyboard, even though there's not enough space

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Simon Bünzli]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5) Gecko/20050929 Firefox/1.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5) Gecko/20050929 Firefox/1.4

When the context menu of an item in a richlistbox is invoked with the keyboard
([App] or [Shift]+[F10]), it is always displayed attached to the bottom border
of the item - even when the item is located at the bottom of the visible screen
space.

Reproducible: Always

Steps to Reproduce:
1. Open the Extensions manager
2. Move it to the bottom of the screen, so that the selected extension is the
last thing you see
3. Hit [Shift]+[F10] (or [App] - just beside the [Win] key)

Actual Results:  
The context menu is cropped to the maximum, although there would've been plenty
of space above the selected extension.

Expected Results:  
The context menu is fully visible.

Comment 1

[Adam Guthrie]

This seems to be the expected behavior after the fix for bug 175568. To me, it
makes sense to show the scroll arrows. Why should we move the context menu up
and put it where it wouldn't normally be?

Comment 2

[Simon Bünzli]

Attachment: wallpaper patch

I doubt that this is related to bug 175568. The problem here seems to be that
with the specific anchoring of the popup, it isn't allowed to move above the
item for fear that it could overlap it (see
http://lxr.mozilla.org/mozilla1.8/source/layout/xul/base/src/nsMenuPopupFrame.cpp#964,
readjustAboveBelow is set to true in AdjustPositionForAnchorAlign above). It
probably should be allowed to move above as long as no overlapping is involved
(which in my reading isn't taken into account in the code mentioned).

The attached wallpaper patch would alleviate the problem pretty painlessly by
positioning the context menu above the richlist item in case below the item is
less than one third of screen space left. A better fix might be provided by bug
279703 - but that won't be before Firefox 2.0.

As for putting the context menu above an item: that's where you'd expect it in
case of not enough space below and enough space above the item it belongs to.
Not having to scroll is better than finding the menu always at the same spot,
especially since this here is just about accessing the menu with the keyboard
where there's no muscle memory about the position of the menu relative to the
mouse cursor involved.

Comment 3

[Simon Bünzli]

This still happens on the current trunk build, although not as regularly as I thought. Steps to reproduce:
1. Open the Extensions manager
2. Shrink it so far that at least the first item from the list is completely hidden
3. Move the window to the lower screen border
4. Select the first item and hit the [App] key
5. Select the last item (make sure it's fully visible) and hit the [App] key

Actual results:
The context menu at step 4 is opened above the item (where expected). At step 5 however, the menu is opened downwards where there might not be enough space.

Expected results:
The context menu always opens above the item if there's not enough space below.

A second way to reproduce a symptom of the same issue:
1. Install a dozen extensions
2. Enlarge the extensions list to show all of them
3. Move the Extensions manager down until most extensions vanish at the bottom of the screen
4. Repeatedly hit [Down] and [App]

Actual results:
The context menu is getting smaller and smaller.

Expected results:
The context menu's size shouldn't depend on the distance between the richlistitem and the border of the visible screen.

Comment 4

[Neil Deakin]

The correct fix here is to remove the bizarre special context menu handling the richlistbox is doing and just rely on the default handling.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

This appears to be fixed following the STR in comment #0 and comment #3. The context menu always appears in a visible place now that the popup rewrite has landed.

Comment 6

[Simon Bünzli]

Well, it's only FIXED if you take the STR by the letter. Adapted STR:

1. Select the first richlistitem and hit [App] or [Shift]+[F10]

Expected: The context menu opens above or below the item (so that the item itself can still be seen).
Actual: The context menu opens over the item.

2. Select the second richlistitem and hit [App]

Expected: Same as above.
Actual: The context menu opens in the very same place as the first item (i.e. relative to the richlistbox and not the richlistitem).

3. Select the first item and move the window to the very bottom of the screen before hitting [App]

Expected: The context menu opens fully visible at the bottom of the screen.
Actual: The context menu may be partially hidden or not visible at all, depending on how long it is and how far below the visible screen the richlistbox is located.

Ryan: Do you want to file new bugs for these issues (in case they haven't yet) or do you prefer morphing this bug?

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Probably better to file a new bug