1750973 - [Fission] TikTok videos not opening from user profiles

Status: VERIFIED FIXED

(Core :: DOM: Navigation, defect)

Description

[x9trw0wk3]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:96.0) Gecko/20100101 Firefox/96.0

Steps to reproduce:

In about:config, "fission.autostart" had set itself to true
I assume I was opted into some experiment

Now:

• Go to tiktok.com
• Navigate to any user profile
• Scroll down
• Click a video
• Close it with the x
• Try to click any other scrolled-down video

Actual results:

The page scrolls back to the top. The video fails to open.

This also happens on a brand new profile created via about:profiles, when fission.autostart is set to true

If fission.autostart is disabled and the browser is restarted, the behavior goes back to normal.

Expected results:

The video should have opened, the page shouldn't change its scroll position.

I.e. the behavior when fission.autostart is disabled

Comment 1

[x9trw0wk3]

Two more users with the issue: https://www.reddit.com/r/firefox/comments/s55d00/videos_from_the_tiktok_profile_in_firefox_do_not/

Comment 2

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

The Bugbug bot thinks this bug should belong to the 'Core::DOM: Navigation' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 3

[Andrew McCreight [:mccr8]]

Fission is enabled by default on all desktop versions of Firefox now, so it isn't an experiment exactly.

Comment 4

[Andrew McCreight [:mccr8]]

I can reproduce the issue. The videos sometimes work when I click on them, but mostly not.

Comment 5

[Andrew McCreight [:mccr8]]

In a debug build, I didn't see any error messages of note with Fission enabled that didn't show up with it disabled.

Another odd thing is that it looks like the user profile pages are only single process, so I'm not sure how the behavior is going to change with Fission.

Comment 6

[Andrew McCreight [:mccr8]]

[Tracking Requested - why for this release]: Seems like a fairly common functionality on TikTok is not working, and was broken by Fission. I'm not exactly sure what the next steps are here, though.

Comment 7

[Andrew McCreight [:mccr8]]

Given that it is a single process page, Nika suggested that this could be a SHIP regression.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Bisected with Fission preffed on:
INFO: Last good revision: c25899d7b631470b983650ee254177381a62eaad (2020-10-02)
INFO: First bad revision: 33139800aca52aac192973fe24c9b74df7c87e00 (2020-10-03)
INFO: Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c25899d7b631470b983650ee254177381a62eaad&tochange=33139800aca52aac192973fe24c9b74df7c87e00

Looks like it was indeed SHIP. Not sure it's worth trying to bisect further back with that force-enabled.

Comment 9

[Andrew McCreight [:mccr8]]

Thanks for doing the bisection, Ryan.

Comment 10

[Peter Van der Beken [:peterv]]

It does use pushState, but that seems to be working correctly. Still debugging.

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

I have a patch but still trying to figure out the testcase.

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Bug 1750973, don't replace the nsILayoutHistoryState object when doing same document history navigations, r=peterv

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b4bbec16312c
don't replace the nsILayoutHistoryState object when doing same document history navigations, r=peterv

Comment 14

[Andreea Pavel [:apavel]]

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

Comment 15

[Ryan VanderMeulen [:RyanVM]]

This seems worth taking as a dot release ride-along given that the fix is reasonably simple, includes tests, and affects a large site. Please nominate for release approval if you agree with that assessment.

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

Yeah, I agree, it might be safe enough. Debugging the issue was extremely tricky and finding a test perhaps even trickier, but the fix is obvious.

Comment 17

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 9262002 [details]
Bug 1750973, don't replace the nsILayoutHistoryState object when doing same document history navigations, r=peterv

Beta/Release Uplift Approval Request

• User impact if declined: Broken behavior in a commonly in a major website
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See the initial comment in the bug
• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): The risk comes from the fact that the patch is changing session history handling. And based
  the experience, those changes tend to be rather regression risky.
  But this seems to be rather edge case which Tiktok somehow manages to rely on.
• String changes made/needed: NA

Comment 18

[Peter Magyari (Desktop QA)]

I have reproduced the issue on Firefox 97.0 and I've verified the fix on Firefox Beta 98.0b4 (20220213185901)

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9262002 [details]
Bug 1750973, don't replace the nsILayoutHistoryState object when doing same document history navigations, r=peterv

Approved for 97.0.1.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

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

Comment 21

[Ryan VanderMeulen [:RyanVM]]

Added to the 97.0.1 release notes.

Fixed an issue where TikTok videos would fail to load when selected from a user's profile page

Comment 22

[Peter Magyari (Desktop QA)]

Verified the fix on Firefox 97.0.1 (20220216142741)