1357780 - Custom Tab: Different color for Share option from menu

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Sorina Florean [:sflorean]]

Attachment: 2017_04_19_17_25_57_Screenshot_Share.png

Environment: 
Device: Huawei Honor (Android 5.1.1);
Build: Nightly 55.0a1 (2017-04-18);

Steps to reproduce:
1. Launch CustomTab Activity;
2. Long tap on any link from the page or image;
3. Observe the share option.

Expected result:
The options have the same color.

Actual result:
The color for menu-options are not the same.

Comment 1

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

[triage]
major identifiable UI flaw

Comment 2

[Julian Chu [:walkingice]]

One of the key points to reproduce this bug is about the toolbar background color, which effects CustomTab to use different theme.

Comment 3

[Julian Chu [:walkingice]]

Attachment: Bug 1357780 - ActionProvider be able to use new context

The Context menu will use ActionProvider to create ActionView to use.
GeckoActionProvider cache a Context inside.

In CustomTabs, we might change Theme at runtime, therefore cached
context might be outdated. Now we check wheter the context is the same
one.

Review commit: https://reviewboard.mozilla.org/r/140486/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/140486/

Comment 4

[Julian Chu [:walkingice]]

Attachment: Bug 1357780 - Popup menu has its own theme

Both of Popup menu and Context menu will use MenuItemDefault.

However in CustomTabs, Popup menu only has one style, regardless light
or dark theme. Now we create one specific theme for it. Wish one day we
can get rid of this.

Review commit: https://reviewboard.mozilla.org/r/140488/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/140488/

Comment 5

[Julian Chu [:walkingice]]

Attachment: Bug 1357780 - To tint state drawawble in MenuItem

The drawable mState in MenuItemDefault is to present state image for
MORE(">"), CHECKED or UNCHECKED.

In CustomTabs there might be dark theme. So we need to tint this
drawable by text-color.

Review commit: https://reviewboard.mozilla.org/r/140490/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/140490/

Comment 6

[Julian Chu [:walkingice]]

Canceled previous patch. I decided just use one Theme(Theme.AppCompat.Light.NoActionBar) in CustomTabs which cause less visual problems. Since CustomTabs re-uses some widgets from Gecko, and there is no dark theme in Gecko. If I apply dark theme in CustomTabs, I might(already) bite myself.

Comment 7

[Julian Chu [:walkingice]]

Attachment: Bug 1357780 - CustomTabs get rid of dark theme

Now we only use Light theme, and its look and feel more close to
Full-Browser. Hopefully when we re-use gecko widgets will have less
visual issue.

In ActionBar, the tint color follows text color. And the text color is
decided by Top toolbar background.

Review commit: https://reviewboard.mozilla.org/r/141122/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/141122/

Comment 8

[Julian Chu [:walkingice]]

Attachment: Bug 1357780 - Fix tint problem in Android 4.4

Use this way to wrap drawable to make tinting works.

Review commit: https://reviewboard.mozilla.org/r/141124/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/141124/

Comment 9

[Julian Chu [:walkingice]]

Comment on attachment 8869530 [details]
Bug 1357780 - CustomTabs get rid of dark theme

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/141122/diff/1-2/

Comment 10

[Julian Chu [:walkingice]]

Comment on attachment 8869531 [details]
Bug 1357780 - Fix tint problem in Android 4.4

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/141124/diff/1-2/

Comment 11

[Julian Chu [:walkingice]]

Comment on attachment 8869530 [details]
Bug 1357780 - CustomTabs get rid of dark theme

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/141122/diff/2-3/

Comment 12

[Julian Chu [:walkingice]]

Comment on attachment 8869531 [details]
Bug 1357780 - Fix tint problem in Android 4.4

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/141124/diff/2-3/

Comment 13

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8869530 [details]
Bug 1357780 - CustomTabs get rid of dark theme

https://reviewboard.mozilla.org/r/141122/#review144882

Comment 14

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8869531 [details]
Bug 1357780 - Fix tint problem in Android 4.4

https://reviewboard.mozilla.org/r/141124/#review144884

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7823d414f9d7
CustomTabs get rid of dark theme r=sebastian
https://hg.mozilla.org/integration/autoland/rev/d5db05712359
Fix tint problem in Android 4.4 r=sebastian

Comment 16

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/7823d414f9d7
https://hg.mozilla.org/mozilla-central/rev/d5db05712359