773088 - Webapps show reading list option in menu

Status: RESOLVED DUPLICATE of bug 777427

(Firefox for Android Graveyard :: Web Apps (PWAs), defect, P1)

Description

[Wesley Johnston (:wesj)]

Saw this the other day and then promptly forgot about it. We're showing readinglist and some other option that was recently added in the menu. Probably need to design a better system for this, so that it isn't so confusing for us. Maybe it would even be good to just have webapp.java specify its own menu resource, different from browserApp's.

Comment 1

[Wesley Johnston (:wesj)]

Attachment: Patch

This is the simplest fix.

Thinking about doing something more... advanced. We could load a separate menu for webapps.... but that means different menus for different layouts (although some of them currently have no way to show a menu) which I hate. Maybe we could hide all these by default and force BrowserApp to unhide them, but thats similar to just moving this code somewhere else.

So in the end I came back to the simple fix. What do you think margaret?

Comment 2

[:Margaret Leibovic]

Comment on attachment 645475 [details] [diff] [review]
Patch

This is a nice low-risk simple fix, but I agree that we should have something in place to prevent this problem in the future.

The fact that we just hide all of these in here makes me think that one solution would be to set android:visible="false" in the XML [1], especially since that's all that this array is used for. What do you think of that? Then BrowserApp can just enable the ones it wants, like you suggested before.

[1] http://developer.android.com/guide/topics/resources/menu-resource.html

Comment 3

[Sriram Ramasubramanian [:sriram]]

Why are we doing this in GeckoApp? Why not we just leave the inflation to BrowserApp?

Comment 4

[Wesley Johnston (:wesj)]

This will be fixed (better) by bug 777427.

Comment 5

[Wesley Johnston (:wesj)]

This was fixed on central by bug 777427. Its implementation is more complex than this (but better long term). I'm tempted to use this on Aurora and it there. I'll nom it tomorrow?