966963 - Update controller.Menu() to work on MetroFirefox

Status: RESOLVED WONTFIX

(Testing Graveyard :: Mozmill, defect)

Description

[Daniel Gherasim]

Given that in the Metro code we have different implementations for Menus and ContextMenus, we need to update the controller.Menu() so that we can use it for MetroFirefox.

Some main differences I observed are :
- the using of popupset, menupopup, popup, menuitem tags that are changed in Metro as box, vbox, richlistbox, richlistitem. 
- the state parameter of the popup is now popupState
- popupshowing event it's named onpopupshowing

Other differences may occur so we need to fix all of them to have the controller.Menu() fully working on Metro.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Can you please point me to some examples of that please?

Comment 2

[Daniel Gherasim]

Sure, here are some examples of how menus are defined in metrofirefox:

Menus:
Metro: http://mxr.mozilla.org/mozilla-central/source/browser/metro/base/content/browser.xul#723
Firefox: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.xul#79

The popup state in Metro menus:
Metro: http://mxr.mozilla.org/mozilla-central/source/browser/metro/base/content/helperui/MenuUI.js#118

Popup showing:
Metro : http://mxr.mozilla.org/mozilla-central/source/browser/metro/base/content/helperui/MenuUI.js#305

Comment 3

[Daniel Gherasim]

I will work on this,

Henrik, given that the menu in metro is defined differently so the code will be different, how do you think we should write this down ? Declaring new methods in controller.Menu like openMetro, closeMetro, _buildMetroMenu and use them in metro tests? or having a new parameter to the Menu object like menuType ( "metro", "firefox" as default) and then check for it's value on each method to run the specific part of code ? or maybe a new MenuMetro object ?

Thanks,

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Please find out why menus and context menus have been implemented like that and why the usual XBL widget do not work. This is kinda suspicious.

Comment 5

[Daniel Gherasim]

Attachment: controllerOpenMenu.txt

(In reply to Henrik Skupin (:whimboo) [away 02/24 - 02/28] from comment #4)
> Please find out why menus and context menus have been implemented like that
> and why the usual XBL widget do not work. This is kinda suspicious.

Context menus and menus still uses the usual XBL bindings:
http://mxr.mozilla.org/mozilla-central/source/browser/metro/base/content/bindings/bindings.xml#82

The main difference here is that we no longer identify a menu by the <menupopup> tag and we have an animation now when we open a contextMenu that we should wait for.

So the menus in metro are defined like this:

<box class="menu-container" id="context-container">
  <vbox class="menu-popup">
    <richlistbox id="id-menu">
      <richlistitem></richlistitem>
      <richlistitem></richlistitem>
      ...
    </richlistbox>
  </vbox>
</box>

I have added an example of how the Menu.open() might be implemented for Metro Firefox.

I think there might be good to find a better way to structure our code for firefox, code that works on metro firefox and for both of them in the future, in mozmill and mozmill-tests too. That would involve some code refactoring so someone may decide if it's worth it.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Setting as wontfix for now. We might want to reconsider to fix it if metro becomes important again for us.