785764 - Work - <select> popup for forms is always in the top left corner of the screen

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: General, defect)

Description

[Jim Mathies [:jimm]]

The container inherits a style that changed with the menu rework in bug 785280. The popup also needs metro styling.

Comment 1

[Matt Brubeck (:mbrubeck)]

It looks like this is basically working now except for one thing: The popup for the select menu is always in the top-left corner.  We should make it use the positioning logic from MenuUI.

Also, we probably don't want to use the popup at all for precise input on <select multiple> or <select size=n> elements (with n > 1).

Comment 2

[Matt Brubeck (:mbrubeck)]

Attachment: WIP

Uses the MenuPopup class to position the SelectHelper menu.  Still has some bugs with clicking items in the menu.

Comment 3

[Matt Brubeck (:mbrubeck)]

Attachment: part 1: Use MenuPopup for positioning

This ports SelectHelperUI to use the MenuPopup class to manage showing/hiding/positioning its popup.  This lets of get rid of quite a lot of code in the process.

I will file follow-up bugs for these two known problems:

* When the menu is too tall, some items are pushed off-screen and there is no way to scroll the menu to see them.

* Clicking with the mouse should on a <select> box with attribute "multiple", or size > 1, should probably not show the touch-friendly popup.

Comment 4

[Jim Mathies [:jimm]]

Comment on attachment 712554 [details] [diff] [review]
part 1: Use MenuPopup for positioning

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

::: browser/metro/base/content/helperui/SelectHelperUI.js
@@ +82,2 @@
>  
> +    //this._scrollElementIntoView(firstSelected);

Did you intend to leave this commented out?

Comment 5

[Matt Brubeck (:mbrubeck)]

(In reply to Jim Mathies [:jimm] from comment #4)
> > +    //this._scrollElementIntoView(firstSelected);
> 
> Did you intend to leave this commented out?

Yes, it's currently broken (throws an exception) and should be fixed with the first follow-up bug mentioned above.  I'll add a comment explaining this.

Comment 6

[Matt Brubeck (:mbrubeck)]

Attachment: part 2: Fix height and scrolling of tall menus

Two things:
1) Set maxHeight/maxWidth to stop menus from overflowing the screen.
2) Since part 1 replaced our <scrollbox> with a <richlistbox>, we can use ensureElementIsVisible and get rid of our DIY version.

Comment 7

[Jim Mathies [:jimm]]

Comment on attachment 712620 [details] [diff] [review]
part 2: Fix height and scrolling of tall menus

more cold code removed - awesome.

Comment 8

[Matt Brubeck (:mbrubeck)]

Folded the two patches together and landed on elm:
http://hg.mozilla.org/projects/elm/rev/6d4766ea292e