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)
Tracking
()
Tracking | Status | |
---|---|---|
firefox111 | --- | verified |
People
(Reporter: dholbert, Assigned: kcochrane)
References
(Blocks 1 open bug)
Details
(Whiteboard: [fidefe-2022-mr1-firefox-view])
Attachments
(1 file)
(deleted),
video/webm
|
Details |
(Spinning this off from bug 1791089).
STR:
- Visit Firefox View; ensure that "Tab Pickup" shows some tabs (from Sync)
- In devtools, manually force your tab-pickup-list to update by executing the following in web console:
document.getElementsByTagName("tab-pickup-list")[0].getSyncedTabData()
- (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.
Reporter | ||
Comment 1•2 years ago
|
||
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.
Reporter | ||
Updated•2 years ago
|
Comment 2•2 years ago
|
||
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?
Updated•2 years ago
|
Reporter | ||
Comment 3•2 years ago
|
||
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.
Comment 4•2 years ago
|
||
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.
Updated•2 years ago
|
Comment 5•2 years ago
|
||
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.
Updated•2 years ago
|
Updated•2 years ago
|
Comment 6•2 years ago
|
||
Dan - Should we clear the needinfo for you on this one?
Reporter | ||
Comment 7•2 years ago
|
||
(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.
Comment 8•2 years ago
|
||
(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.
Updated•2 years ago
|
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Comment 9•2 years ago
|
||
This ticket should be resolved by changes in bug 1791089
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Comment 10•2 years ago
|
||
Making sure QA tests that this was also fixed by bug 1791089
Reporter | ||
Comment 11•2 years ago
|
||
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)
Reporter | ||
Updated•2 years ago
|
Comment 12•2 years ago
|
||
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.
Description
•