1625913 - Card image gets cut off for some widths

Status: VERIFIED FIXED

(Firefox :: Messaging System, defect, P1)

Description

[Andrei Oprea [:andreio]]

Attachment: Screen Shot 2020-03-30 at 12.38.51.png

I think we want the background-size to be cover instead.

Comment 1

[Valentina Virlics [:vvalentina], Ecosystem QA]

Attachment: card.png

Comment 2

[Valentina Virlics [:vvalentina], Ecosystem QA]

Comment on attachment 9136738 [details]
card.png

This issue is also reproduced for: 
- Stay Alert to Data Branches
- Create and Store Strong Passwords

Comment 3

[Valentina Virlics [:vvalentina], Ecosystem QA]

Attachment: card2.png

Comment 4

[Emily McMinn :emcminn]

Attachment: Bug 1625913 - Fix for onboarding card image widths in smaller resolutions

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/06d53e1ac917
Fix for onboarding card image widths in smaller resolutions r=andreio

Comment 6

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/06d53e1ac917

Comment 7

[Valentina Virlics [:vvalentina], Ecosystem QA]

I have verified this issue using Firefox Nightly 76.0a1 (Build ID 20200401212659) on Windows 10x64, Mac OS 10.15 and Linux Ubuntu 18.04 and it is still reproducible in about:newtab page.
Please see attached screenshot.

Comment 8

[Valentina Virlics [:vvalentina], Ecosystem QA]

Attachment: cards cut.png

Comment 9

[Emily McMinn :emcminn]

Attachment: Bug 1625913 - Added fix for card images to activity-stream.css

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3203a945e08f
Added fix for card images to activity-stream.css r=andreio

Comment 11

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/3203a945e08f

Comment 12

[Emily McMinn :emcminn]

Hi Valentina, would you mind verifying this for us again? The fix should now cover both about:welcome and about:newtab. We're going to request uplift once it's been verified. Thanks!

Comment 13

[Valentina Virlics [:vvalentina], Ecosystem QA]

I have verified this using Firefox Nightly 77.0a1 (Build ID 20200407093520) on Windows 10 x64 and observed the following:

• about:welcome - Stay Alert to Data Breaches card image is not cut;
• about:newtab
  • Stay Alert to Data Breaches card image is not cut;
  • Instantly Send Yourself Tabs card image is not cut;
  • Create and Store Strong Passwords card image is cut at full-screen browser dim (attaching a screenshot)

Comment 14

[Marius Coman [:mcoman], Ecosystem QA]

Based on the above comment I am changing the status to Verified.

Comment 15

[Emily McMinn :emcminn]

So after some investigation by Andrei, we're going to re-write this patch a bit. Turns out the card image for Lockwise (create and store strong passwords) has slightly different dimensions than the others, which is causing this issue. We're going to go back to the spec and change how we're handling the image background/sizing to make sure that images of any size are placed correctly.

Comment 16

[Emily McMinn :emcminn]

Attachment: Bug 1625913 - Changes to handling of card image sizes

Comment 17

[Emily McMinn :emcminn]

Attachment: Bug 1625913 - Background size and padding fix for Onboarding cards

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:emcminn, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 19

[Valentina Virlics [:vvalentina], Ecosystem QA]

Attachment: strng pass.png

As I mentioned in comment 13, the "Lockwise" card is still cut.

- Create and Store Strong Passwords card image is cut at full-screen browser dim (attaching a screenshot)

Based in comment 15 and comment 13, I'll reopen this issue.

Comment 20

[Emily McMinn :emcminn]

(In reply to Valentina Virlics [:vvalentina], Ecosystem QA from comment #19)

Created attachment 9140328 [details]
strng pass.png

As I mentioned in comment 13, the "Lockwise" card is still cut.

- Create and Store Strong Passwords card image is cut at full-screen browser dim (attaching a screenshot)

Based in comment 15 and comment 13, I'll reopen this issue.

We have a new patch coming, just in review now :)

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cc092011bfe6
Background size and padding fix for Onboarding cards r=andreio

Comment 22

[Emily McMinn :emcminn]

Hi again Valentina :P Hopefully this is the last time this needs verification. We're going to request uplift once it's been verified. Thanks!

Comment 23

[Noemi Erli[:noemi_erli]]

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

Comment 24

[Valentina Virlics [:vvalentina], Ecosystem QA]

I have verified that the issue is no longer reproducible on Firefox Nightly 77.0a1 Build ID 20200415215103 using Win 10, Linux Ubuntu 18.04 and Mac 10.15.
The "Create and Store Strong Passwords" card image is no longer cut.

Comment 25

[Emily McMinn :emcminn]

Comment on attachment 9139533 [details]
Bug 1625913 - Background size and padding fix for Onboarding cards

Beta/Release Uplift Approval Request

• User impact if declined: Users will see incorrectly cut off card images at some screen widths.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Small CSS change that affects only the onboarding cards in about:newtab and about:welcome.
• String changes made/needed: none

Comment 26

[Ryan VanderMeulen [:RyanVM]]

Does the patch from comment 11 also need uplifting?

Comment 27

[Emily McMinn :emcminn]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #26)

Does the patch from comment 11 also need uplifting?

Nope, that one will actually now be obsolete.

Comment 28

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9137830 [details]
Bug 1625913 - Added fix for card images to activity-stream.css

Right, but that patch won't graft cleanly if we don't take the other one first. I'm approving both under that assumption, but feel free to yell loudly if I'm misunderstanding.

Comment 29

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a510b3be48ea
https://hg.mozilla.org/releases/mozilla-beta/rev/078045810e55

Comment 30

[Emily McMinn :emcminn]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #28)

Comment on attachment 9137830 [details]
Bug 1625913 - Added fix for card images to activity-stream.css

Right, but that patch won't graft cleanly if we don't take the other one first. I'm approving both under that assumption, but feel free to yell loudly if I'm misunderstanding.

Oh, yes of course you're right. I'm going to blame Friday.

Comment 31

[Valentina Virlics [:vvalentina], Ecosystem QA]

I have verified that this is no longer reproducible on Firefox Beta 76.0b6 (Build ID 20200420031429) using Windows 10 x64, Linux Ubuntu 18.04 and Mac OS 10.15.
No image of any cards from about:newtab - Onboarding Triplets, gets cut.