Closed Bug 1791098 Opened 2 years ago Closed 2 years ago

The Firefox View "Tab Pickup" section's "Last Active" badge blink/flash/change size when Tab Pickup list is re-rendered/updated

Categories

(Firefox :: Firefox View, defect, P3)

Desktop
All
defect
Points:
3

Tracking

()

VERIFIED FIXED
Tracking Status
firefox111 --- verified

People

(Reporter: dholbert, Assigned: kcochrane)

References

(Blocks 1 open bug)

Details

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

Attachments

(1 file)

(Spinning this off from bug 1791089).

STR:

  1. Visit Firefox View; ensure that "Tab Pickup" shows some tabs (from Sync)
  2. In devtools, manually force your tab-pickup-list to update by executing the following in web console:
document.getElementsByTagName("tab-pickup-list")[0].getSyncedTabData()
  1. (optional) Repeat step 2.

ACTUAL RESULTS:
Every time the tab pickup list updates, there's a brief visual glitch -- the "Last Active" badge briefly collapses away to be a small circle and then comes back.

EXPECTED RESULTS:
No such visual glitch.

Right now this happens at every step of the tour as noted in bug 1791089. But I assume it also happens in the background when the synced tab list is updated, too.

Attached video screencast of bug (deleted) —

Here's a video of the STR, where I run the statement over and over in devtools (every second or so). You can see "Last Active" badge collapsing away each time I run the statement.

The very first time I ran the statement (around 0:02), the favicons on the tiles disappear as well, just as in my screenshot on bug 1791089.

I didn't notice that happening after the first time, so I think that might be a different issue -- that part might be a subtler image handling thing.

Blocks: firefox-view

The badge thing is because fluent (to get the "last active" string) is async and for whatever reason layout/painting manages to happen between the badge being created and inserted into the DOM with its fluent ID already set, and fluent resolving the "new" bit of l10n and populating its textContent. "In theory" this was meant to never happen when fluent was designed, or "not matter because it's only a very short delay", but in practice, there are annoying side effects like this. You can also see this e.g. when opening macOS dialogs where the buttons have text populated by fluent - the dialogs resize on opening because the text gets put in "too late".

This is very frustrating but honestly, I think should be fixed at a lower level than individual frontend components that run into this (though of course they can in principle work around... it just makes the code ugly, all of the time). It shouldn't be the case that populating l10n from files that have already loaded and content that was already translated affects visual layout in this way because "everything should be async".

I don't know what the deal is with the favicons. That feels like a layout/painting question though. The images we're using have clearly been loaded before - why are we painting before they're ready?

Flags: needinfo?(dholbert)
Whiteboard: [fidefe-2022-mr1-firefox-view]

I don't know what the deal is with the favicons. That feels like a layout/painting question though. The images we're using have clearly been loaded before - why are we painting before they're ready?

I don't know, offhand. I'd probably have to capture a recording & take a look in pernosco to better understand what's going on there.

tnikkel is closer to our image-painting code & might have ideas here, too; CC'ing him as a FYI or in case he wants to take a look.

So far I haven't been able to reproduce the favicon blink. Something that could narrow down where the problem is is making RasterImage::Discard a no-op. If that doesn't fix it the issue is more likely to be gecko code.

Severity: -- → S3
Priority: -- → P2

The favicons are using moz-anno:favicon and if I had to guess I expect the caching story for those types of URLs isn't great (in contrast to "normal" http or similar URLs). If there's an obvious way of improving the situation for the favicons we can do it, otherwise I'll spin it off into its own bug. That said - like :tnikkel I can't seem to reproduce the favicons disappearing at all. Was this a debug build, by any chance?

I've changed bug 1791089 to capture avoiding the update entirely if the contents aren't changing, which will help with the general problem. We didn't bother initially because for 3 cards it's hardly worth the code that the performance optimization would take... Anyway, I think in this bug I'd like to cover avoiding the reflow of the last active badge, though really I'd prefer it if fluent didn't suck, I guess we can write a band-aid for it here by re-jigging how the frontend creates that badge.

OS: Unspecified → All
Hardware: Unspecified → Desktop
Summary: The Firefox View "Tab Pickup" section's favicons (and "Last Active" badge) briefly collapse out of existence when Tab Pickup section is refreshed → The Firefox View "Tab Pickup" section's favicons and "Last Active" badge blink/flash/change size when Tab Pickup list is re-rendered/updated
Priority: P2 → P3
Blocks: 1797520
No longer blocks: firefox-view

Dan - Should we clear the needinfo for you on this one?

(In reply to Timothy Nikkel (:tnikkel) from comment #4)

So far I haven't been able to reproduce the favicon blink.

I'm not seeing it right now either (though I think it was always a bit hard to see or trigger; it was lucky that I caught it in the screencast I think).

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

Was this a debug build, by any chance?

It was a regular Nightly (not a debug build).

I've changed bug 1791089 to capture avoiding the update entirely if the contents aren't changing, which will help with the general problem.

Thanks!

We didn't bother initially because for 3 cards it's hardly worth the code that the performance optimization would take... Anyway, I think in this bug I'd like to cover avoiding the reflow of the last active badge,

Sounds good. That part I can definitely still repro/see with STR in comment 0.

Flags: needinfo?(dholbert)

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

The favicons are using moz-anno:favicon and if I had to guess I expect the caching story for those types of URLs isn't great (in contrast to "normal" http or similar URLs). If there's an obvious way of improving the situation for the favicons we can do it, otherwise I'll spin it off into its own bug. That said - like :tnikkel I can't seem to reproduce the favicons disappearing at all. Was this a debug build, by any chance?

I've changed bug 1791089 to capture avoiding the update entirely if the contents aren't changing, which will help with the general problem. We didn't bother initially because for 3 cards it's hardly worth the code that the performance optimization would take... Anyway, I think in this bug I'd like to cover avoiding the reflow of the last active badge, though really I'd prefer it if fluent didn't suck, I guess we can write a band-aid for it here by re-jigging how the frontend creates that badge.

So it looks like we will still plan to surface a "Last Active" badge in the updated design as well. That being said, I think this is something worth fixing. Agreed that we can focus this bug on the badge concern only.

Summary: The Firefox View "Tab Pickup" section's favicons and "Last Active" badge blink/flash/change size when Tab Pickup list is re-rendered/updated → The Firefox View "Tab Pickup" section's "Last Active" badge blink/flash/change size when Tab Pickup list is re-rendered/updated
Points: --- → 3
Assignee: nobody → kcochrane
Status: NEW → ASSIGNED
Blocks: 1791089
No longer blocks: 1791089
Blocks: 1791089

This ticket should be resolved by changes in bug 1791089

No longer blocks: 1791089
Depends on: 1791089

Making sure QA tests that this was also fixed by bug 1791089

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Flags: qe-verify+
Flags: needinfo?(alexandru.trif)
Resolution: --- → FIXED

This seems fixed to me.

I retested comment 0's STR using my main browsing profile (cloned automatically by mozregression), with these mozregression-launched Nightly versions (straddling bug 1791089's fix):
2023-02-02: bug is reproducible ("Last Active" badge briefly collapses when repeatedly executing the command in web console)
2023-02-03: bug is not reproducible. ("Last Active" badge doesn't collapse; no visible rendering change)

Status: RESOLVED → VERIFIED
Flags: needinfo?(alexandru.trif)

Thank you, Daniel! Confirming the fix as well on Windows 10x64, macOS 12, and Ubuntu 20.04 by using Firefox 111.0a1 (20230205211244). The Last Active badge no longer flashes after following STR from comment 0.

Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: