1873019 - [User Journey] After refresh about welcome page becomes unfunctional

Status: VERIFIED FIXED

(Firefox :: Messaging System, defect, P1)

Description

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

Attachment: about-welcome-refresh.png

Found in

• Firefox 122.0b5

Affected versions

• Firefox 122.0b5
• Firefox 121.0
• Firefox 120.0

Affected platforms

• Windows 10, Ubuntu 22, macOS 12

Preconditions

• Firefox IS NOT set as the default browser.

Steps to reproduce

1. Launch Firefox with new profile.
2. Have the first screen of the "about:welcome" page displayed.
3. Observe the elements displayed on the page, checkboxes are functional (can be checked/unchecked).
4. Click on "Skip this step" until you reach the Start browsing page.
5. Refresh the about:welcome page.

Expected result

• The checkboxes from the first slide are functional and clicking on "Skip this step" button the next slide appears.

Actual result

• The checkboxes from the first slide are not functional and clicking on "Skip this step" button appears an empty slide.

Regression range

• I will come back with regression range ASAP.

Additional notes

• Latest Firefox Nightly not affected.
• See this issue in the attached screenshot.

Comment 1

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

:zstimi, if you think that's a regression, could you try to find a regression range using for example mozregression?

Comment 2

[Daniel Bodea [:danibodea]]

This is not reproducible in Firefox Beta v115.0b2 or Beta v117.0b3 and it appears that the about:welcome slides are different so this is most probably an issue since the latest changes to these slides.

A similar issue would be reproduced starting in Beta v119.0b1-v119.0b7. Moreover, a webcontent freeze could be observed after refreshing the about:welcome page process in the tab with the "Start browsing" button.

This being said, this issue started reproducing in Beta v119.0b1. Considering it cannot be reproduced in Nightly, an exact regressor can not be provided.

Comment 3

[Punam Dahiya [:pdahiya]]

Possible fallout of Bug 1855031 that landed in Fx119 , triaging for current iteration to debug and investigate underlying issue

Comment 4

[Donal Meehan [:dmeehan]]

Setting the regressor based on Comment 3.
:pdahiya can you review the severity? Sounds worse than an S4?
Will we have an investigation in time for Fx122, we are in the last week of beta

Comment 5

[Negin]

I'm not sure if this is related to easy setup changes which landed in 120.0a1. While use mozregression I noticed a "No permissions for local state folder." error which was implemented in D188874. I built that patch and was able to reproduce the error without the easy setup changes (although my console is unfortunately frozen). I'll attach a recording of the bug. Interestingly, the old import screen renders on refresh instead of embedded migration wizard which makes me wonder if this might be a permission related bug?

Sorry for all the pings this week Mike, but curious if you have any context on why this error could be throwing/causing about:welcome to freeze? I can confirm that I'm not able to reproduce this issue in Nightly and will try to dig into when this regression may have been resolved (and started).

edit: It seems like removing embedded migration wizard screen still causes the regression - so I'm not sure if it's related to a permission issue and maybe something related to our screen targeting (specifically (EVALUATE_SCREEN_TARGETING)) but still curious what causes "No permission for local state folder" to log. As a sanity check, I also removed the easy setup screens and I can still reproduce the freeze.

Also given that this issue started in 119.0b1 I don't believe 1855031 or 1818237 may have caused this since they both landed in 120.0a1. Still narrowing down when this issue started.

Comment 6

[Negin]

Attachment: AW crash on reload

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Something fairly bizarre is happening here. I can reproduce on Beta, and what appears to be happening is that JS starts running in the privileged about content process, and it gets really busy: https://share.firefox.dev/3vo2Xmi

We've hit some kind of degenerate case here. Still investigating.

Comment 8

[Mike Conley (:mconley) (:⚙️)]

This profile includes screenshots, so it's easier to see what's happening when: https://share.firefox.dev/48LJ07l

Comment 9

[Negin]

Thanks for the profiler link! I can also reproduce the issue by refreshing the second slide https://share.firefox.dev/48PyKez on 119.0b1. Looks like a ton of re-renders are happening after refreshing on the 2nd slide. I logged out a counter and after refresh it goes from 14 re-renders -> thousands of re-renders until it ultimately freezes. Looking into what might be causing these astronomical re-renders.

Comment 10

[Shane Hughes [:aminomancer]]

Mozregression got me to this range:
https://hg.mozilla.org/releases/mozilla-beta/pushloghtml?fromchange=45c770ff73e393c4ca15eb188d0b68d3ee091ad8&tochange=704d786ef571a57be79a4e8bbf4b3d348da4a6af

I tried git bisect on this range. All builds were good until I reached the top 5 commits, when mach build started failing.

Maybe this means the problem is not reproducible in local builds and is caused by something in integration. I don't really know how these releng treescripts work though.

Comment 11

[Ed Lee :Mardak]

At least in one local beta debugger session, it seems like setScreens is continuously being called because languageFilteredScreens keeps changing. If I set a breakpoint inside useLanguageSwitcher and hitting play/resume then checking screens shows it keeps switching between an array of 6 items and an array of 9 items. So the effect keeps triggering itself.

https://searchfox.org/mozilla-central/rev/6f90f50b7a32cc062ab755e0653b3d3f512fe3bd/browser/components/aboutwelcome/content-src/components/MultiStageAboutWelcome.jsx#197-207

Comment 12

[Ed Lee :Mardak]

Unclear if related but the nightly behavior is reloading the page keeps the progression, e.g., reload from mobile screen shows mobile screen again. But beta seems to be restarting from index 0.

Comment 13

[Punam Dahiya [:pdahiya]]

Testing Dec 16th nightly build which is current code base in beta doesn't replicate the hung state seen on refresh pointing to issue could be due to difference in build settings between beta and nightly wondering if there are recent react settings differences between two builds. @dmeehan considering the issue impacts user flow on refresh but still an edge case, bumping up the severity to S3. Thanks

Comment 14

[Negin]

(In reply to Ed Lee :Mardak from comment #11)

At least in one local beta debugger session, it seems like setScreens is continuously being called because languageFilteredScreens keeps changing. If I set a breakpoint inside useLanguageSwitcher and hitting play/resume then checking screens shows it keeps switching between an array of 6 items and an array of 9 items. So the effect keeps triggering itself.

https://searchfox.org/mozilla-central/rev/6f90f50b7a32cc062ab755e0653b3d3f512fe3bd/browser/components/aboutwelcome/content-src/components/MultiStageAboutWelcome.jsx#197-207

This seems like the likely culprit, although I don't know why there seems to be a discrepancy between Nightly and Beta (unless there are no language packs compatible with nightly?)

I logged out the this useEffect and it keeps firing. I logged out the dependency but on 119.0b1 it logs out the same array of 8 elements. Commenting out language switcher screen from AW resolves the issue but it's not obvious to me yet why the dependency is changing (or how) and what caused this to start happening in Fx119+.

Comment 15

[Negin]

I think I may have found a potential regression https://bugzilla.mozilla.org/show_bug.cgi?id=1849882. Reverting the dependency changes in LanguageSwitcher.jsx resolves the infinite re-renders. But patch seems to have landed in 118.0a1 while the regression seems to have started 119.01b (but in 118 I did notice an issue where it would render the incorrect screen on refresh after the 2nd slide) so I'm a bit confused about that. I don't see any other obvious changes to LanguageSwitcher or MultiStageAboutWelcome that could cause this regression. Maybe a use case for useCallback if this is deemed the cause of the regression?

Will verify if I can reproduce the issue in 118.0a1.

Comment 16

[Ed Lee :Mardak]

(In reply to Negin from comment #14)

discrepancy between Nightly and Beta (unless there are no language packs compatible with nightly?)

Oh, that's a good point. I don't think langpacks are normally available on nightly unless you specially set some prefs. Bug 1849882 adding additional effect dependencies means the effect can run much more often than previously configured potentially resulting in multiple languageFilteredScreens changes?

Comment 17

[Negin]

(In reply to Ed Lee :Mardak from comment #16)

(In reply to Negin from comment #14)

discrepancy between Nightly and Beta (unless there are no language packs compatible with nightly?)

Oh, that's a good point. I don't think langpacks are normally available on nightly unless you specially set some prefs. Bug 1849882 adding additional effect dependencies means the effect can run much more often than previously configured potentially resulting in multiple languageFilteredScreens changes?

Yeah that's my guess! I re-introduced each reverted dependency since logging them out didn't reveal anything. Re-introducing screenIndex dependency is the only one causing infinite re-renders but when I log out the value it's 1 so that's a bit odd https://searchfox.org/mozilla-central/source/browser/components/aboutwelcome/content-src/components/LanguageSwitcher.jsx#142

setScreenIndex(screenIndex - 1) should mean that it's not creating a new object so I'm not sure what exactly is causing it to re-compute?

Comment 18

[Negin]

Attachment: Bug 1873019 - Remove dependency from useEffect in LanguageSwitcher causing infinite re-renders

Comment 19

[Punam Dahiya [:pdahiya]]

[Tracking Requested - why for this release]:

Impacts User experience causing about:welcome page to become unresponsive after user navigates to second screen and hit refresh

Comment 20

[Negin]

Attachment: Bug 1873019 - [Beta] Remove dependency from useEffect in LanguageSwitcher causing infinite re-renders

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/07e2b829af43
Remove dependency from useEffect in LanguageSwitcher causing infinite re-renders r=omc-reviewers,Mardak,aminomancer

Comment 22

[Negin]

Comment on attachment 9372309 [details]
Bug 1873019 - Remove dependency from useEffect in LanguageSwitcher causing infinite re-renders

Requesting uplift on the beta patch to avoid conflicts with D197884.

Beta/Release Uplift Approval Request

• User impact if declined: Users who refresh about:welcome on 2nd slide onward and then try to navigate back/forth will result in an unresponsive browser.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Steps can be found in the phabricator patch
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Reverted a change from 4 months ago which was a useEffect dependency change in LanguageSwitcher in 1849882 causing the initial regression
• String changes made/needed:
• Is Android affected?: No

Comment 23

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/07e2b829af43

Comment 24

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

From the beginning I didn't manage to reproduce this issue on nightly (nor on nightly 118.0a1), so I can't validate the fix on latest nightly 123.0a1.
Is there any way I could check that the bug is fixed in latest nightly 123.0a1?

Comment 25

[Negin]

(In reply to Timea Zsoldos [:zstimi/tzsoldos], Desktop QA from comment #24)

From the beginning I didn't manage to reproduce this issue on nightly (nor on nightly 118.0a1), so I can't validate the fix on latest nightly 123.0a1.
Is there any way I could check that the bug is fixed in latest nightly 123.0a1?

Sorry for the confusion. I don't believe there's a way to validate this on Nightly as the issue is not reproducible on Nightly (unless there's a way to get valid language packs?) I have a beta patch that has the same fix and is what I'd like to be uplifted and ultimately verified as it's the same solution but reproducible on beta. cc'ing Shane/Ed in case there's a way to validate this on Nightly that I'm not aware of.

Comment 26

[Donal Meehan [:dmeehan]]

Just a note, we are at the end of the beta cycle so this uplift won't make a beta build.
However, it can be included in 122.0 RC1.

Comment 27

[Negin]

(In reply to Donal Meehan [:dmeehan] from comment #26)

Just a note, we are at the end of the beta cycle so this uplift won't make a beta build.
However, it can be included in 122.0 RC1.

No worries, thanks for letting me know! 122.0 RC1 sounds good to me given how late into this cycle the patch got in.

Comment 28

[Ed Lee :Mardak]

(In reply to Timea Zsoldos [:zstimi/tzsoldos], Desktop QA from comment #24)

From the beginning I didn't manage to reproduce this issue on nightly (nor on nightly 118.0a1)

Does setting intl.multilingual.aboutWelcome.languageMismatchEnabled to true cause the problem without the fix?

Comment 29

[Negin]

(In reply to Ed Lee :Mardak from comment #28)

(In reply to Timea Zsoldos [:zstimi/tzsoldos], Desktop QA from comment #24)

From the beginning I didn't manage to reproduce this issue on nightly (nor on nightly 118.0a1)

Does setting intl.multilingual.aboutWelcome.languageMismatchEnabled to true cause the problem without the fix?

Oh that works for me - reverted the change on Nightly and set that to true and I can reproduce the error. I'll update the test plan, thanks Ed!

Comment 30

[Donal Meehan [:dmeehan]]

Comment on attachment 9372335 [details]
Bug 1873019 - [Beta] Remove dependency from useEffect in LanguageSwitcher causing infinite re-renders

Approved for 122.0 RC1

Comment 31

[Donal Meehan [:dmeehan]]

Comment on attachment 9372309 [details]
Bug 1873019 - Remove dependency from useEffect in LanguageSwitcher causing infinite re-renders

See https://bugzilla.mozilla.org/attachment.cgi?id=9372335

Comment 32

[Pulsebot]

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

Comment 33

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

Reproduced on Firefox nightly from 2024.01.04 with intl.multilingual.aboutWelcome.languageMismatchEnabled = true the browser became unresponsive. Verified on latest nightly 123.0a1 (2024.01.14) the browser is responsive.
Will verify on Firefox 122.0 RC1 when will be available.

Comment 34

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

Verified on Firefox 122.0 using Windows 10, Ubuntu 22 and macOS 12, the browser is responsive.