1398362 - (Photon) Image/logo size in onboarding screen is incorrect on API 16 & 17

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[:Grisha Kruglov]

Attachment: Screenshot_1504907299.png

I don't have an API16 device, but I see this behaviour in the emulator. See screenshot.

Other screens appear to be correct.

Comment 1

[:Grisha Kruglov]

Flagging for triage, and marking as 57 blocker.

Comment 2

[Wesly Huang (EPM)]

[triage0913] + as P1. btw the logo needs to be updated anyway once 57 become beta, per 1399378.

Comment 3

[Julien Cristau [:jcristau]]

This was described as "57 blocker" over 2 weeks ago.  Is anyone looking at this?

Comment 4

[Wesly Huang (EPM)]

(Sorry for late, Julien)

Jingwei, would you help on this by Oct. 11? 

Maybe this is fixed already by https://bugzilla.mozilla.org/show_bug.cgi?id=1399378? Thanks.

Comment 5

[Liz Henry (:lizzard) (relman/hg->git project)]

The icon is reported to be missing in a couple of places still in bug 1399378 and bug 1388679. 

Ioana, maybe you can help here to check over where we expect the logo to appear in Fennec?

Comment 6

[Jing-wei Wu [:jwu]]

Attachment: Screenshot of Nightly on API 16 emulator

The attachment is the screenshot of current Nightly on API 16 emulator, we should already fix this issue. Ioana, please help confirm it on real devices, thanks.

Comment 7

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Wesly, Grisha, is there a fix for this in the works? It is tracked as a recent regression in 57 and a fix for this would meet the beta57 uplift bar.

Comment 8

[Wesly Huang (EPM)]

Since we have other bugs also addressing the logo icon (fixed around the end of Sep., while this issue is reported on Sep. 8th) I believe this bug is now fixed by one of them already (as checked by comment#6), so no further patch is planned for this bug currently.

Comment 9

[Julien Cristau [:jcristau]]

(In reply to Wesly Huang (Fennec Frontend/Firefox EPM) from comment #8)
> Since we have other bugs also addressing the logo icon (fixed around the end
> of Sep., while this issue is reported on Sep. 8th) I believe this bug is now
> fixed by one of them already (as checked by comment#6), so no further patch
> is planned for this bug currently.

That doesn't seem to address 57?

Comment 10

[Wesly Huang (EPM)]

Good point Julien, my bad. Though the two bugs[1][2] dressing the logo are both uplifted to 57 Beta already, it's good to be 100% sure.

@Grisha, since Jingwei left Mozilla last week, would you kindly help check this in the emulator again, if you don't have such device? Thanks.

@Ioana, will be good if you have an actual device (with API 16, JB) to check as well, thanks!


[1] bug 1399378
[2] bug 1388679

(In reply to Julien Cristau [:jcristau] from comment #9)
> (In reply to Wesly Huang (Fennec Frontend/Firefox EPM) from comment #8)
> > Since we have other bugs also addressing the logo icon (fixed around the end
> > of Sep., while this issue is reported on Sep. 8th) I believe this bug is now
> > fixed by one of them already (as checked by comment#6), so no further patch
> > is planned for this bug currently.
> 
> That doesn't seem to address 57?

Comment 11

[:Grisha Kruglov]

I've checked on an API16 emulator. I see the image, on-boarding looks fine.

Comment 12

[Wesly Huang (EPM)]

Sorry, Grisha, I should be more specific and just to be sure that we've addressed the concern, you were testing on "57 Beta", right?

Thanks Grisha!(In reply to :Grisha Kruglov from comment #11)
> I've checked on an API16 emulator. I see the image, on-boarding looks fine.

Comment 13

[Wesly Huang (EPM)]

Nevin, though not reported by you I think you'll be able to help get the same environment (API 16 emulator with 57 Beta) to test it? Would you help?

Comment 14

[Nevin Chen(Not active on Bugzilla)]

Attachment: firstrun.png

I think the problem is solved here. The logo is diplayed on API 16.
However I found an additional slightly(?) issue.

Since we are using adjustViewBounds[1] in firstrun page, there's a difference between API 17- and 18+[2]. The result will make firstrun_welcome.png (the logo) smaller in API 17-. See attachment for example.

[1] http://searchfox.org/mozilla-central/rev/d30462037ffea383e74c42542c820cf65b2b144e/mobile/android/app/src/main/res/layout/firstrun_basepanel_checkable_fragment.xml#27
[2] https://developer.android.com/reference/android/widget/ImageView.html#setAdjustViewBounds(boolean)

Comment 15

[Wesly Huang (EPM)]

Thanks Nevin for the quick check! 

Considering the impact (only in the onboarding tour) and the Android user distribution on different versions[1] (Joe maybe you have the numbers for Fennec users?) I personally don't think this is a blocker for 57 (nor 58), but will let UX and Product to make the final say. Thanks.

[1] https://developer.android.com/about/dashboards/index.html

Comment 16

[Carol Huang [:Carol]]

Hi Nevin,
As offline discussion, please replace the image size as big as the one of other onboarding page to see if we could solve this problem. Thanks!

Comment 17

[Nevin Chen(Not active on Bugzilla)]

Since the logo did appears but with the wrong size, I'll update the summary.

Comment 18

[Nevin Chen(Not active on Bugzilla)]

Attachment: Bug 1398362 - Change the image with correct size.

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

Comment 19

[Nevin Chen(Not active on Bugzilla)]

Attachment: firstrun_fixed.png

After this patch, it will look like this:

Comment 20

[Carol Huang [:Carol]]

It looks better! thank you, Nevin!

Comment 21

[Max Liu [:maliu]]

Comment on attachment 8921342 [details]
Bug 1398362 - Change the image with correct size.

https://reviewboard.mozilla.org/r/192362/#review197526

Comment 22

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8921342 [details]
Bug 1398362 - Change the image with correct size.

https://reviewboard.mozilla.org/r/192362/#review197536

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/238d4670044e
Change the image with correct size. r=maliu

Comment 24

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8921342 [details]
Bug 1398362 - Change the image with correct size.


Approval Request Comment
[Feature/Bug causing the regression]: Android has different implementations in different api level
[User impact if declined]: user will see a small logo
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]:no
[Is the change risky?]:no
[Why is the change risky/not risky?]: Just change icon
[String changes made/needed]:no

Comment 25

[Wesly Huang (EPM)]

Hi Ritu:

Per offline discussion with the team we agreed this is not a 57 hard blocker, but will be good to include if there is going to have another 57 beta (beta 12 maybe?), and Nevin just made the uplift request (the fix is to update the logo file so considered low risk). 

Sorry for the inconvenience and please feel free to let me know shall you have any concern/question. Thanks.

Comment 26

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Thanks for the note Wesly. Fixing this is a good idea and low risk, let's take it.

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/b1f8fcfb3260

Comment 28

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/238d4670044e

Comment 29

[Ioana Chiorean]

Verified Fixed on LG D722 with Android 4.4.2 with all channels - Release 57.0. Beta 58.0b3 and Nightly 59.