Closed Bug 1234544 Opened 9 years ago Closed 9 years ago

The menu_panel_bg drawable is gone

Categories

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

46 Branch
defect
Not set
normal

Tracking

(firefox46 fixed)

RESOLVED FIXED
Firefox 46
Tracking Status
firefox46 --- fixed

People

(Reporter: sunhaitao, Assigned: sunhaitao)

References

Details

Attachments

(3 files, 1 obsolete file)

Open Web Apps on Android still have menus spawn from hardware button. Without the menu_panel_bg drawable, their background become transparent.
Attached patch menu_panel_bg.patch (obsolete) (deleted) — Splinter Review
Assignee: nobody → sunhaitao
Status: NEW → ASSIGNED
Attachment #8701051 - Flags: review?(s.kaspari)
Comment on attachment 8701051 [details] [diff] [review] menu_panel_bg.patch mcomella is probably the best one for reviewing this. I don't remember the details but didn't we have a problem with some devices using dark backgrounds and some others using light backgrounds?
Attachment #8701051 - Flags: review?(s.kaspari) → review?(michael.l.comella)
(In reply to SUN Haitao from comment #0) > Open Web Apps on Android still have menus spawn from hardware button. > Without the menu_panel_bg drawable, their background become transparent. I think the "most correct" solution is to backout the bug that changed this, bug 1229958. Thanks for the report, Sun! (In reply to Sebastian Kaspari (:sebastian) from comment #2) > I don't remember the > details but didn't we have a problem with some devices using dark > backgrounds and some others using light backgrounds? This was a side effect of using a color as the panelBackground, rather than a drawable. iirc, AppCompat necessitated this but if the web apps don't use AppCompat, maybe the menu just works.
Status: ASSIGNED → NEW
Actually, I see we performed some funny business on GB in bug 1229958, where it was defined as: <item name="android:panelBackground">@color/text_and_tabs_tray_grey</item> Note that the issue Sebastian mentioned in comment 2 was specific to GB devices, where we can't always override the menu color. Sun, did you test this fix on GB devices?
Depends on: 1229958
Flags: needinfo?(sunhaitao)
Comment on attachment 8701051 [details] [diff] [review] menu_panel_bg.patch Review of attachment 8701051 [details] [diff] [review]: ----------------------------------------------------------------- lgtm, provided this works on GB.
Attachment #8701051 - Flags: review?(michael.l.comella) → review+
Looking at alternative solutions, what does the hardware menu button spawn on web apps, Sun? Perhaps you can post a screenshot? Can software-only menu button users also get these menus? Do we need to change the mechanisms (as we did on the full browser) to make the hardware menu button work like the software menu button and avoid the transparency issue altogether?
Attached image Menu on Nightly (deleted) —
Flags: needinfo?(sunhaitao)
Attached image Menu on Patched (deleted) —
I haven't test this on GB yet. It is a little bit hard to find such devices around here now.
(In reply to SUN Haitao from comment #9) > I haven't test this on GB yet. It is a little bit hard to find such devices > around here now. Can you give me an STR? I have a few devices I can test on.
Flags: needinfo?(sunhaitao)
Steps to Reproduce: 0. Install and launch an Open Web App with Fennec. 1. Press the hardware 'menu' button. 2. A menu with transparent background will appear.
Flags: needinfo?(sunhaitao)
Where do I get an Open Web App from? Are those the web apps from marketplace.firefox.com?
Flags: needinfo?(sunhaitao)
(In reply to Michael Comella (:mcomella) from comment #12) > Where do I get an Open Web App from? Are those the web apps from > marketplace.firefox.com? We are dropping support for those web apps, so I don't think we should spend time worrying about this: https://bugzilla.mozilla.org/show_bug.cgi?id=1235869 What menu is even expected to be shown? Web apps shouldn't rely on hardware menu buttons being present.
(In reply to Michael Comella (:mcomella) from comment #12) > Where do I get an Open Web App from? Are those the web apps from > marketplace.firefox.com? Yes, they are. > We are dropping support for those web apps, so I don't think we should spend > time worrying about this: > https://bugzilla.mozilla.org/show_bug.cgi?id=1235869 I am building a Gecko-based Crosswalk-like solution with some friends. We chose to reuse the WebRT for GeckoView seems not quite mature. Currently we have got a working prototype with some minor patches, and planning a real-world trail in next Spring. Dropping WebRT may cause us a lot of trouble. Maybe we could discuss this matter more on Bug 1235869/Mailing List? > What menu is even expected to be shown? Web apps shouldn't rely on hardware > menu buttons being present. The current implementation provides a non-customizable menu to 'Quit' the OWA.
Flags: needinfo?(sunhaitao)
(In reply to SUN Haitao from comment #14) > I am building a Gecko-based Crosswalk-like solution with some friends. We > chose to reuse the WebRT for GeckoView seems not quite mature. Currently we > have got a working prototype with some minor patches, and planning a > real-world trail in next Spring. Dropping WebRT may cause us a lot of > trouble. Maybe we could discuss this matter more on Bug 1235869/Mailing List? Yes, we can discuss this matter in the discussion forum, and that's the best place to discuss it, since that's where we're having the conversation about disabling the web runtimes for desktop and Android.
I tested on GB and we need the panelBackground definition there too. Since we don't want to leave open web apps in a broken state before we remove it, let's land this with the GB change.
The hardware-button-triggered menu of Open Web App is still using it.
Assignee: sunhaitao → michael.l.comella
Status: NEW → ASSIGNED
Comment on attachment 8703173 [details] [diff] [review] Add the menu_panel_bg drawable back Review of attachment 8703173 [details] [diff] [review]: ----------------------------------------------------------------- I already r+'d this.
Attachment #8703173 - Flags: review+
I filed bug 1236071 to remove panelBackground if open web apps are removed.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: