1612108 - Onboarding Triplets can be wrongly dismissed from about:welcome page while the onboarding modal is still displayed

Status: VERIFIED FIXED

(Firefox :: Messaging System, defect, P1)

Description

[Valentina Virlics [:vvalentina], Ecosystem QA]

[Affected versions]:

• Firefox 73.0b11 Build-ID 20200128001646

[Affected Platforms]:

• Windows 10
• Mac 10.14
• Linux Debian 9

[Steps to reproduce]:

1. Open about:welcome page.
2. Observe the top-right side of the page.

[Expected result]:

• A grayed out and unactionable “X” button is displayed.

[Actual result]:

• An actionable “X” button is displayed.

[Regression]:

• Last good revision: 63822634f6ffb878bd3aa64c1789b3452d178bee
• First bad revision: 0152641c5419db9c9e723afe55bef1b63c699e96
  Pushlog:
  https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=63822634f6ffb878bd3aa64c1789b3452d178bee&tochange=0152641c5419db9c9e723afe55bef1b63c699e96

[Additional notes]:

• The onboarding Triplets are dismissed if the “X” button is clicked.
• This issue was fixed on Firefox Nightly 74.0a1 (Build 20200129093157) by Bug 1609462.

@thecount, @pdahiya could you please take a look at Bug 1609462 and see if it will be safe to be uplifted in Beta?

Comment 1

[Scott [:thecount] Downe]

I'm a bit confused how regress is being used when describing 1609462, but it sounds like, and based on my own testing, that 1609462 fixes the issue, and is not the regression, is that correct?

Also, I'm not sure if it's worth uplifting this, seems a little on the late side, and the patch seems a little on the large side. But, If other's are comfortable with the uplift or the UX gain from it is critical, I think it can be done.

Comment 2

[Valentina Virlics [:vvalentina], Ecosystem QA]

As I mentioned above in "Additional notes", Bug 1609462 fixed the issue and the pushlog from the Regression section lists the changeset (and bug) where this issue was introduced: Bug 1602914 .

If we let this slide and get into Release, we run the risk of not showing the Triplets cards and extended Triplets for new users.
@tspurway, @abenson, could you please weigh in on this?

Comment 3

[Punam Dahiya [:pdahiya]]

Thanks for logging the issue, I will look into the fix and see if we can use 1609462 patch or another simpler fix for uplift

Comment 4

[Punam Dahiya [:pdahiya]]

It appears the correct regressing bug is 1589038, adding back z-index on .modalOverlayOuter fixes the issue. Both 1589038 and 1602914 landed on same day probably thats why the confusion. NI QA to verify regression range and update, Thanks

Comment 5

[Punam Dahiya [:pdahiya]]

@emcminn @mardak Do you foresee any issue if we add back z-index here https://searchfox.org/mozilla-beta/source/browser/components/newtab/content-src/asrouter/components/ModalOverlay/_ModalOverlay.scss#18, that will be probably easiest uplift.

Comment 6

[Emily McMinn :emcminn]

The z-index was part of what was causing the original issue with the scrollbars, but I remember we also tried to simplify the CSS for the overlay so we may have removed it in too many places. I can take a look and see if adding it back in that one spot works for both cases :)

Comment 7

[Valentina Virlics [:vvalentina], Ecosystem QA]

@pdahiya
We run Mozregression again in order to recheck regression range, on Win and Mac and seems like you are right.
New [regression] range is:
Last good revision: 2c035b2f4f5a878b403a401b0d54556d53936a13
First bad revision: 0d526197ba712c52528f7e2f9680764afa0d87df
Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=2c035b2f4f5a878b403a401b0d54556d53936a13&tochange=0d526197ba712c52528f7e2f9680764afa0d87df

Comment 8

[Emily McMinn :emcminn]

Attachment: Bug 1612108 - z-index fix for ModalOverlay

Quick regression fix for uplift.

Comment 9

[Emily McMinn :emcminn]

Comment on attachment 9123638 [details]
Bug 1612108 - z-index fix for ModalOverlay

Beta/Release Uplift Approval Request

• User impact if declined: Risk of not showing the Triplets cards and extended Triplets for new users
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Open about:welcome & verify that the dismiss button for Triplets (upper right corner) is greyed out and unclickable.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Single line CSS change to z-index - Tested on beta and it fixes issue reported in 73. This issue is not replicable in central and this custom patch is for beta only.
• String changes made/needed: none

Comment 10

[Tim Spurway [:tspurway]]

[Tracking Requested - why for this release]: This is fixed in Nightly but is still broken in beta. Risk of not showing the Triplets cards and extended Triplets for new users.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9123638 [details]
Bug 1612108 - z-index fix for ModalOverlay

Fixes an issue which can cause the onboarding triplets to not be properly displayed. Approved for 73.0RC1.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/b9bf9b05c08b

Comment 13

[Valentina Virlics [:vvalentina], Ecosystem QA]

I have verified that the issue is no longer reproducible on Firefox Nightly 74.0a1 (Build ID 20200203214717) and Firefox Beta 73 (Build ID 20200203203546) using Win 10, Mac 10.14 and Linux Debian 9.
Onboarding Triplets cannot be dismissed from about:welcome page.