1012896 - [B2G][Open_C][E-mail]The Reply, Reply All, and Forward icons are cut off

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Description

[Andrew Stole [:astole]]

Attachment: logcat

The Reply, Reply All, and Forward icons in the email app appear to be too large and are cut off

Repro Steps:
1) Update a Open_C to BuildID: 20140519040204
2) Open the Email app
3) Select an email
4) Select Reply

Actual:
The icons in the Reply option are too large and get cut off

Expected:
The icons do not get cut off

2.0 Environmental Variables:
Device: Open_C 2.0 MOZ
BuildID: 20140519040204
Gaia: 101c500903a2477f9de1ea5ce523b9e0be4d45d0
Gecko: 41a54c8add09
Version: 32.0a1
Firmware Version: P821A10v1.0.0B06_LOG_DL

Repro frequency: 100%
See attached: Screenshot, logcat

Comment 1

[Andrew Stole [:astole]]

This issue does not occur on Master on Buri. The icons are not cut off on the Buri device.

2.0 Environmental Variables:
Device: Buri 2.0 MOZ
BuildID: 20140519040204
Gaia: 101c500903a2477f9de1ea5ce523b9e0be4d45d0
Gecko: 41a54c8add09
Version: 32.0a1
Firmware Version: V1.2-device.cfg

Adding qawanted to check Base on Open_C

Comment 2

[Andrew Stole [:astole]]

Attachment: Screenshot

Comment 3

[Brogan Zumwalt [Inactive]]

Issue does NOT occur on Open C 1.3 Base image P821A10V1.0.0B06_LOG_DL

Reply, Reply All, and Forward icons display full icon as expected with Gmail and Outlook accounts that were manually set up.

Comment 4

[Jason Smith [:jsmith]]

UX might care to get this fixed as part of the visual refresh. I think this is a hidpi regression between 1.3 --> 1.4.

UX - What image assets should be used here?

Comment 5

[Stephany Wilkes]

Flagging Patryk to address the asset question and to determine if this should be added to the Visual Refresh list.

Comment 6

[James Burke [:jrburke]]

I am going to dupe this to an existing bug that is for checking all the icons for the visual refresh, bug 1010673. That bug was just updated within the last 24 hours, and I expect this issue may be solved by that update. If not, I will work with Fang in that other bug to get updated icons.

Comment 7

[James Burke [:jrburke]]

Oh hmm, if this is for 1.4, then maybe we keep the bug open, and I can selectively backport some of those icons for a 1.4 bug. So reopening.

Comment 8

[Andrew Sutherland [:asuth] (he/him)]

I suspect it's not an icons issue but a style issue.  The inclusion of the icons in the action menu I think was an ad-hoc thing :mcav did when creating the action menu.  We're probably not forcing the larger image sizes to have smaller display size or something like that.

Comment 9

[Marcus Cavanaugh [:mcav]]

Yeah, this is probably just a mistake on my part, not adding a background-size or something in the action menu. :jrburke, feel free to assign to me if you'd rather not investigate.

Comment 10

[James Burke [:jrburke]]

So this issue is just a scaling one, as :asuth suspected. Specifically, for hidpi builds, for some of the email icons, background-size is not set to the size of the icon space that is desired.

I have fixed these cases in the visual refresh branch for bug 950208. There was more than just this menu that needed fixing, and there are some incorrect hidpi (@2x) images also.

So if the 1.4 branch will be delivered to hidpi devices (where the build sets GAIA_DEV_PIXELS_PER_PX=2), then we likely need a bigger patch that changes some other CSS and replaces some of the images. We can collect those images from the the bug 950208 work, but work up a custom changeset for 1.4.

If 1.4 will not go to hidpi devices, then this bug should be closed out as a dupe of bug 950208.

Comment 11

[James Burke [:jrburke]]

Attachment: GitHub pull request

Looking more into it, the Flame device is a 1.4 device that uses the @1.5x icons. So the only change that needs to be done for the 1.4 branch is to make sure that all icons have a background-size set on them.

This patch does it, and fixes some icon cases for the drawer too. The drawer is only on master though. I will prepare a custom 1.4 patch that only does the non-drawer updates if this bug gets targeted to land for 1.4.

Comment 12

[Marcus Cavanaugh [:mcav]]

Comment on attachment 8426666 [details] [review]
GitHub pull request

Tested on my flame, the icons look right.

Comment 13

[James Burke [:jrburke]]

Merged from gaia master:
https://github.com/mozilla-b2g/gaia/commit/fc8de79b79a0a10822594342dc44ed89da996abb

from pull request:
https://github.com/mozilla-b2g/gaia/pull/19509

Also asking for 1.4? since this affects Flame and any other non-@1x devices that will use 1.4. I will need to generate a custom patch for the 1.4 branch in that case.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Needs a branch patch for v1.4 uplift.

Comment 15

[James Burke [:jrburke]]

Merged in 1.4:
https://github.com/mozilla-b2g/gaia/commit/8f4201a44676eb70926a3d2645d94bf92fcd6718

from pull request:
https://github.com/mozilla-b2g/gaia/pull/19604