It would be pretty darn neat (and remove a lot of java code complexity) if you could add context menu items to the native UI. Something along the lines of: NativeWindow.contextmenus.add("Test", NativeWindow.URLBAR, function() { alert("Test"); }); or more advanced (part 2?): NativeWindow.contextmenus.add("Test", NativeWindow.AWESOMEBAR_ITEM.when(function(item) { return item.url == myURL; }, function() { alert("Test"); }); We can still be very careful and only expose UI pieces that we want. Functions like the "when" bit above would likely vary for different UI pieces?

Just a beginning piece for this. Doesn't all work yet.

This seems to work well for me.

And a second little patch to show how easy it is to add new native ui chunks.

Grr. I am going back to all mc. Trying to juggle repos is not something I am good at.

Comment on attachment 728381 [details] [diff] [review] Patch v1 Missing NativeWindow.java ? I need to think a little more about the round trip that happens with "NativeWindow:ContextMenu" and "NativeWindow:ContextMenu:Result"

Whoops. Added NativeWindow.java. I am a bit nervous about the message-return style passing here as well. In practice (on my S3), Gecko returns quickly enough that everything is seamless, but there is a race. For instance, if I do this during startup, before Gecko is ready, the menu items are still added (once gecko is started), but they appear "below the fold" and you have to scroll to see them. There's a flash of the scroll bar when they're added to indicate something happened, but that's it. I looked a bit for a way to tell Java to resize the menu, but didn't see anything. Alternatively, we could store a list of the items in Java and use that fill things in. That makes things like "Show this action only on Google sites" harder to do (but we could still allow passing up a regexp or something...). Its also more book-keeping for addons as they might have to actively enable/disable even if the user never opens the context menu.

Comment on attachment 729151 [details] [diff] [review] Patch In general I like this approach. I think we need to on the way we make distinction between different native contextmenus. Some nits: * NativeWindow.contextmenu.urlBar * NativeUI("urlbar")

Comment on attachment 728380 [details] [diff] [review] Patch - Add awesomebar screen This one will need re-worked with the new Home page. Instead of "AWESOMEBAR_ROW", I think we'd want: * "historyitem", "bookmarkitem", etc...

Refreshed patch. This works NativeWindow.java a bit differently. Here NativeWindow just holds a HashMap mapping native ui elements to a hashtable of menuitems that can be shown. Syntax is something like: var id = NativeWindow.contextmenus.add("Test", NativeWindow.contextmenus.urlbar, callback); var id = NativeWindow.contextmenus.add("Test", NativeUI("URLBAR"), callback); NativeWindow.contextmenus.remove(id);

Wrong patch.

Adds the ability to add some "states" to check before showing the item. i.e. var id = NativeWindow.contextmenus.add("Test", NativeUI("URLBAR").addState("url", ".*google.*"), callback); NativeWindow.contextmenus.remove(id); will only show on pages that include the word "google" somewhere in their url. We could add more state functions for things in the awesomebar like "bookmarked" or in the tabs tray like "selected"? Also, I should note that I named this NativeWindow because I'm hoping we can move more of our menu registration code in here and 1.) Unify all the paths we have to add menu items, bringing a uniform set of features across them. Context menus should be able to support checked items or submenus even? 2.) Fix up some of the application lifetime issues we have. Currently, items added to menus are stored in the menus themselves, which means they follow the Activity lifetime, not the Applications. Since these are stored on a static version of NativeWindow, they should follow the Application lifecycle.

friendly review ping

My recent traumatic reaction to letting JS have too much control over the Java UI is making me want to avoid this set of patches. 1. Are we adding this for add-ons? I don't think we want to move our own UI from Java to JS. Java is where the explicit UI should be. 2. Is picking a specific part of the Java UI to attach the menu (using URLBAR, AWESOMEBAR_ITEM or HISTORY or BOOKMARK) too narrowly focused? Should we think about Actions.jsm where Actions could be for Page, History or Bookmarks, and then the Java side will display the actions when the context is right. For some reason I am not getting a good feeling for this feature.

It also adds complexity to our contextmenu system, an already complex system.

(In reply to Mark Finkle (:mfinkle) from comment #14) > 1. Are we adding this for add-ons? I don't think we want to move our own UI > from Java to JS. Java is where the explicit UI should be. Yes, this is mostly for addons. Although, I don't mind if some of our actions are in js. Many of them don't apply until Gecko has been loaded anyway. Many of them just send a message to js to tell it to do something as soon as they're clicked. > 2. Is picking a specific part of the Java UI to attach the menu (using > URLBAR, AWESOMEBAR_ITEM or HISTORY or BOOKMARK) too narrowly focused? Should > we think about Actions.jsm where Actions could be for Page, History or > Bookmarks, and then the Java side will display the actions when the context > is right. I think I would rather that these drive context's if we're going to go that route. i.e. we already have this somewhat. "Open in new tab" applies to things that have uri's assoicated with them. "Pause" applies if we have media that's not playing. Ideally, we'd have one "Share" action that was queried and shown in situations where the object looks like something sharable. That makes more sense to me in our embedding context, where Page, History, or Bookmarks might not apply. And if Java can register "Actions" as well, we may not have to update 6 places every time we change share anymore... We already partially have this system in place. i.e. context menu items in Gecko register themselves not on UI elements, but for what contexts they apply in. We could try to send that information to Java. So for instance, an image embedded in a link might have: { type: "Action:Add", title: "Open in new tab", applies: {IMAGE: {}, URL: {OPENABLE}}, } (playing with some sort of parent-child relationship here to simplify...) and then in Java places where we show context menus we could query: MenuItem[] items = Actions.get(EnumSet(URL_OPENABLE)); That feel any better? > For some reason I am not getting a good feeling for this feature. I don't know what this means. Its been in pretty high demand from add-on authors.

Comment on attachment 777633 [details] [diff] [review] Patch I don't know where this bug is going, but its not going there any time soon I guess. I have some different ideas about how to do this now based on registering "Actions" from JS that can show up in a number of different places....

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.