Closed Bug 1792698 Opened 2 years ago Closed 2 years ago

The Last Active tab from Firefox View does not update the title for Youtube pages if different videos are reached in the same tab without switching away from it

Categories

(Firefox :: Firefox View, defect, P3)

Desktop
All
defect

Tracking

()

VERIFIED FIXED
108 Branch
Tracking Status
firefox105 --- unaffected
firefox106 --- wontfix
firefox107 --- wontfix
firefox108 --- verified

People

(Reporter: rdoghi, Assigned: markh)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [fidefe-2022-mr1-firefox-view])

Attachments

(2 files)

Attached video SyncFxView.mp4

Found in

  • 106.0b5 (2022-09-28)

Affected versions

  • Firefox Nightly 107.0a1
  • Beta 106.0b5

Affected platforms

  • all

Steps to reproduce

  1. On Machine A Sign in with an FxA account and Reach Firefox View.
  2. On Machine B Sign in with the same FxA account and reach Youtube.com
  3. On Machine B search for Cooking and Select any video from the list.
  4. Click Sync Now on both devices from the FxA panel.
  5. On Machine B select a different video from the Suggestions on the Right.
  6. Click Sync Now on both devices from the FxA panel.

Expected result

  • The Subsuquent videos from Machine B have different page titles and they should update for Machine A Firefox view Tab pickups section (Last Active)
    Actual result

  • The Youtube page titles do not update in the Firefox View Tab pickups section on Machine A.

  • Please note that if the user clicks any other Tab or Firefox View and then back to the Youtube tab on Machine B after syncing again it will imediatelly update the title for the Last Active tab in the Firefox View of Machine A.

Please note that in the Attached Screen recording Wikipedia page titles will update in real time after each sync without any issues but on youtube it will only update after clicking away from the tab and then back on the youtube tab and sync.

I have also waited over 30 minutes and it wouldnt update in Firefox View If I never click away from the Youtube tab.
In the attached video I tried 2 different browsers with 2 separate fresh profiles but it also occurs with different machines using Nightly on both.

Has STR: --- → yes

This seems like it'd be a sync-specific issue rather than a Firefox View issue. Title for tabs are all updated in the same way, so its a matter of what data is supplied.

Priority: -- → P3
Whiteboard: [fidefe-2022-mr1-firefox-view]

Thanks Sarah! To confirm...it sounds like you think this is a sync specific issue? Could the website data also potentially be the problem as well? Looping in Janet just in case.

Flags: needinfo?(jdragojevic)

Thanks for the bug report. It would be super helpful if you could please follow the instructions here: https://wiki.mozilla.org/CloudServices/Sync/File_a_desktop_bug#Get_detailed_Sync_logs to grab some sync logs so we can hopefully see what's going on.

Feel free to NI me when they are attached.

Flags: needinfo?(jdragojevic) → needinfo?(rares.doghi)

I can reproduce this. The problem is that the notification we get here has the LOCATION_CHANGE_SAME_DOCUMENT flag set (and only that flag - ie, the flag value is 1), so we ignore it. This is almost certainly because youtube is using the pushState API. This optimization is to avoid re-syncing when pages do "internal" reloads etc, so I'm a little reluctant to just drop this flag, but also can't see a good option for detecting this case otherwise.

OTOH though, this means we are also ignoring changes to the "fragment" (the url changing from "http://foo.com/page#foo" to "http://foo.com/page#bar") will also not be synced.

Gijs, do you have any suggestions here? Are you aware of a way we can detect this via some other observer? Should we consider dropping the check for that flag and see what catches fire?

Flags: needinfo?(rares.doghi) → needinfo?(gijskruitbosch+bugs)

(In reply to Mark Hammond [:markh] [:mhammond] from comment #5)

I can reproduce this. The problem is that the notification we get here has the LOCATION_CHANGE_SAME_DOCUMENT flag set (and only that flag - ie, the flag value is 1), so we ignore it. This is almost certainly because youtube is using the pushState API. This optimization is to avoid re-syncing when pages do "internal" reloads etc, so I'm a little reluctant to just drop this flag, but also can't see a good option for detecting this case otherwise.

OTOH though, this means we are also ignoring changes to the "fragment" (the url changing from "http://foo.com/page#foo" to "http://foo.com/page#bar") will also not be synced.

Gijs, do you have any suggestions here? Are you aware of a way we can detect this via some other observer? Should we consider dropping the check for that flag and see what catches fire?

Ugh, sorry, this needinfo got a bit lost in my giant pile of them. :-(

I have no real suggestions other than removing the check for this flag. If you want to, you could be conservative and capture changes to the URL but ignore hash changes by removing the flag but checking if the URL you had on record before is equalsExceptRef to the new URL (which would mean youtube changes lead to changes, but #foo to #bar changes on what is otherwise the exact same URL would not).

Does that answer your question?

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(markh)
Blocks: 1797520
Blocks: 1788692

(In reply to :Gijs (he/him) from comment #6)

I have no real suggestions other than removing the check for this flag.

Given our quick-sync mechanism, I think ignoring LOCATION_CHANGE_SAME_DOCUMENT is an optimization that isn't saving us much in practice - simply changing which tab has focus causes us to sync all tabs, so ignoring notifications with this flag set probably isn't going to cause any noticeable increase in how often we sync in practice, and honoring them makes it far more likely the URL will be accurate (ie, fragments would be correct and it fixes this pushState scenario)

If you want to, you could be conservative and capture changes to the URL but ignore hash changes by removing the flag but checking if the URL you had on record before is equalsExceptRef to the new URL (which would mean youtube changes lead to changes, but #foo to #bar changes on what is otherwise the exact same URL would not).

We don't actually know what the URL was before, so can't trivially make this check (and as above, knowingly ignoring a fragment change seems wrong anyway). What we could do is to store what we last uploaded to the server (ie, the entire set of tabs), and only perform the upload if we would be writing something different - but I suspect that would not save much either given just changing tabs means the order changes even if URLs have not.

Does that answer your question?

It does, thanks. I think I'll just put up a patch that stops checking that flag, and use our telemetry to check that any increase in syncing is marginal (or not at all noticeable) and either revert it or otherwise mitigate it in the unlikely case it turns out to be bad.

Flags: needinfo?(markh)
Assignee: nobody → markh
Status: NEW → ASSIGNED
Pushed by mhammond@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3e8677afa571
stop ignoring LOCATION_CHANGE_SAME_DOCUMENT notifications when deciding whether to sync tabs. r=skhamis
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 108 Branch

The patch landed in nightly and beta is affected.
:markh, is this bug important enough to require an uplift?

  • If yes, please nominate the patch for beta approval.
  • If no, please set status-firefox107 to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(markh)

This issue is verified as fixed in our latest Nightly 108.0a1 (2022-11-04).

Flags: needinfo?(markh)

Hi, Since 107 is marked with Wont Fix should we update the flags for Release 106 as well ? or is it possible that it will be uplifted to Release ?

Flags: needinfo?(markh)
Flags: needinfo?(markh)

Updating the remaining flags, Thank you @Mark H

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: