1023914 - Empty state image cut off

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[:Margaret Leibovic]

ibarlow ran into this: http://cl.ly/image/451j2x2s2k0Z

Looks like a regression from bug 996708. That patch isn't mission critical, so we could just back it out to start.

Comment 1

[Lucas Rocha (:lucasr)]

(In reply to :Margaret Leibovic from comment #0)
> ibarlow ran into this: http://cl.ly/image/451j2x2s2k0Z
> 
> Looks like a regression from bug 996708. That patch isn't mission critical,
> so we could just back it out to start.

Is this bug Nightly-specific? It seems like the fix would be to simply set scaleType to fitCenter.

Comment 2

[:Margaret Leibovic]

Attachment: Turn empty view image padding into margin

The problem is that there was padding taking up part of the image height. Since this space only exists to put space between the image and the text, replacing the padding with a margin fixes the issue.

Comment 3

[:Margaret Leibovic]

(In reply to Lucas Rocha (:lucasr) from comment #1)
> (In reply to :Margaret Leibovic from comment #0)
> > ibarlow ran into this: http://cl.ly/image/451j2x2s2k0Z
> > 
> > Looks like a regression from bug 996708. That patch isn't mission critical,
> > so we could just back it out to start.
> 
> Is this bug Nightly-specific? It seems like the fix would be to simply set
> scaleType to fitCenter.

I would prefer to just have the image match the size of the ImageView, since the point of the patch in bug 996708 was to provide guidelines about what image size developers should use. However, I suppose we could also change the scaleType to prevent developers from accidentally ending up with cut off images if they use the wrong image size. What do you think?

Comment 4

[Lucas Rocha (:lucasr)]

(In reply to :Margaret Leibovic from comment #3)
> (In reply to Lucas Rocha (:lucasr) from comment #1)
> > (In reply to :Margaret Leibovic from comment #0)
> > > ibarlow ran into this: http://cl.ly/image/451j2x2s2k0Z
> > > 
> > > Looks like a regression from bug 996708. That patch isn't mission critical,
> > > so we could just back it out to start.
> > 
> > Is this bug Nightly-specific? It seems like the fix would be to simply set
> > scaleType to fitCenter.
> 
> I would prefer to just have the image match the size of the ImageView, since
> the point of the patch in bug 996708 was to provide guidelines about what
> image size developers should use. However, I suppose we could also change
> the scaleType to prevent developers from accidentally ending up with cut off
> images if they use the wrong image size. What do you think?

I agree about setting guidelines for add-on developers but I think we should try to ensure the UI doesn't look too broken if guidelines are not followed. fitCenter would at least ensure we never show cut off images. I don't feel strongly about it, up to you.

Comment 5

[:Margaret Leibovic]

Also updated the scaleType.

https://hg.mozilla.org/integration/fx-team/rev/0137da4202c5

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/0137da4202c5

Comment 7

[Teodora Vermesan (:TeoVermesan)]

Attachment: Screenshot_2014-06-16-14-36-50.png

Device: Alcatel One Touch
OS: Android 4.1.2
Build: Firefox for Android 33.0a1 (2014-06-16)
The empty state image is not cut anymore.

Comment 8

[:Margaret Leibovic]

Comment on attachment 8439628 [details] [diff] [review]
Turn empty view image padding into margin

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 996708 
User impact if declined: empty view image cut off
Testing completed (on m-c, etc.): landed on m-c 6/15
Risk to taking this patch (and alternatives if risky): low-risk tweaks to layout for empty add-on panels
String or IDL/UUID changes made by this patch: none

Comment 9

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8439628 [details] [diff] [review]
Turn empty view image padding into margin

Aurora approval granted.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1765a031396e

Comment 11

[Teodora Vermesan (:TeoVermesan)]

Device: LG Nexus 4 (Android 4.4.2)
Build: Firefox for Android 32.0a2 (2014-06-19)
The empty state image is not cut anymore.