1488720 - Quick share icons are too large

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Theme and Visual Design, defect)

Description

[Oana Horvath [:ohorvath - out of office]]

Attachment: Screenshot_20180905-144745.png

Devices:
Google Pixel C Android 8.0.0
Samsung Galaxy S8+ Android 8.0.0
OnePlus 3 (Android 6.0.1)
Nexus 9	(Android 6.0.1)
*NR: Devices with Android 5

Build: Nightly 63.0a1 (2018-08-05)

Steps to reproduce:
1. Open a webpage.
2. Open the custom menu, select Share and tap on a sharing option.
3. Re-open the custom menu and observe the quick share icons.

Expected result:
Regular size icons.

Actual result:
Icons are too large (see screenshot).

Regression window:
Last good build: 2018-08-29
First bad build: 2018-08-30

Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b75561ff5ffec3164338952adfe58620e5e3bc1d&tochange=815b46ed118286d682ce258e6147df5dc691eabf

DEBUG : Found commit message:
Bug 1486200 - Apply Photon theme for Date/Time pickers only on API >=23; r=jchen

Fix Lollipop crashes because of android.content.res.ColorStateList.addFirstIfMissing()
The default app theme will be applied for the material Date/Time pickers on
API 21 and API 22. After this, on API >=23 they will be Photon themed.

Comment 1

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1488720 - [Android M+] improve themes.xml; r=sdaswani

The issue in this ticket was because of missing menuItemSecondaryActionBarStyle,
property of the menu items shown in the SecondaryActionBar.

The GeckoAppBase style from values-v23/themes.xml contained the properties
initially defined in the general values/themes.xml.
Now changed to contain the properties from the closest values-v21/themes.xml.

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 9006556 [details]
Bug 1488720 - [Android M+] improve themes.xml; r=sdaswani

Jim Chen [:jchen] [:darchons] has approved the revision.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c1dfab9dd166
[Android M+] improve themes.xml; r=jchen

Comment 4

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/c1dfab9dd166

Comment 5

[Oana Horvath [:ohorvath - out of office]]

Verified as fixed on Nightly 64 (2018-09-07).
Devices:
Google Pixel (Android 9)
OnePlus 5T (Android 8.0)

Comment 6

[Pascal Chevrel:pascalc (PTO until Sept 2)]

Petru, can you make a patch for an uplift to beta and request that uplift? Thanks

Comment 7

[Petru-Mugurel Lingurar [:petru]]

Comment on attachment 9006556 [details]
Bug 1488720 - [Android M+] improve themes.xml; r=sdaswani

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1486200
[User impact if declined]: Small visual defect - Icons in the share menu appear slightly larger.
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: --
[List of other uplifts needed for the feature/fix]: --
[Is the change risky?]: No
[Why is the change risky/not risky?]: Little change, has been verified in Nightly.
[String changes made/needed]: --

Comment 8

[Petru-Mugurel Lingurar [:petru]]

Comment on attachment 9006556 [details]
Bug 1488720 - [Android M+] improve themes.xml; r=sdaswani

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1486200
[User impact if declined]: Small visual defect - Icons in the share menu appear slightly larger.
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: --
[List of other uplifts needed for the feature/fix]: --
[Is the change risky?]: No
[Why is the change risky/not risky?]: Little change, has been verified in Nightly.
[String changes made/needed]: --

Comment 9

[Pascal Chevrel:pascalc (PTO until Sept 2)]

Comment on attachment 9006556 [details]
Bug 1488720 - [Android M+] improve themes.xml; r=sdaswani

Uplift approved for 63 beta 5

Comment 10

[Julien Cristau [:jcristau] (back August 18)]

Comment on attachment 9006556 [details]
Bug 1488720 - [Android M+] improve themes.xml; r=sdaswani

IMO a "small defect" doesn't warrant dot release inclusion

Comment 11

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/releases/mozilla-beta/rev/078e8bfee2d1

Comment 12

[Andrei Bodea[:andrei]]

Verified as fixed on latest Beta build - 63.0b5.
Device: Google Pixel C(Android 8.0.0). Following the steps from the description.

Comment 15

[Jan Henning [:JanH]]

(In reply to Julien Cristau [:jcristau] from comment #10)
> Comment on attachment 9006556 [details]
> Bug 1488720 - [Android M+] improve themes.xml; r=sdaswani
> 
> IMO a "small defect" doesn't warrant dot release inclusion

Depends - does this count as small?

Comment 16

[Jan Henning [:JanH]]

This: https://i.imgur.com/fTyWic0.jpg

Comment 17

[Julien Cristau [:jcristau] (back August 18)]

Comment on attachment 9006556 [details]
Bug 1488720 - [Android M+] improve themes.xml; r=sdaswani

I guess not.  I was going by the description in comment 12.

Approved for 62.0.2

Comment 18

[Julien Cristau [:jcristau] (back August 18)]

https://hg.mozilla.org/releases/mozilla-release/rev/169867270ec5

Comment 19

[Stefan Deiac]

Verified as fixed on the latest version of Firefox Release (62.0.2) with OnePlus 3 (Android 6.0.1) and Nokia 6 (Android 7.1.1).

Comment 20

[Ralf]

Attachment: Wrong icon sizes in share link popup

Not sure if it is related but the popup shows wrong icon sizes when using the share link menu. See screenshot.

How to reproduce: long press on a link and select the share button.

Android 8.1 / Firefox 62.0.2

Comment 21

[Stefan Deiac]

(In reply to Ralf from comment #20)
> Created attachment 9011205 [details]
> Wrong icon sizes in share link popup
> 
> Not sure if it is related but the popup shows wrong icon sizes when using
> the share link menu. See screenshot.
> 
> How to reproduce: long press on a link and select the share button.
> 
> Android 8.1 / Firefox 62.0.2

The bug 1397174 it's filed for your issue, if you want to add some information please write it there, thanks.