1779714 - [Colorway Closet] about:addons theme page flickers during selection in the modal

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect, P3)

Description

[Mehmet]

Attachment: video of the issue.mov

Nightly 104.0a1 (2022-07-14) (64-Bit)
macOS

1.) Enable Colorway Closet
2.) Open the Colorway Closet modal
3.) Click through Soft/Balanced/Bold in the modal
4.) Take a look at the about:addons theme page beneath the modal during step 3

Actual: about:addons theme page flickers.

Expected: about:addons theme page should not flicker.

A video of the issue is attached.

Thanks for checking :)

Comment 1

[Luca Greco [:rpl] [:luca] [:lgreco]]

Hi :dao,
may I ask you what would be the priority of this issue from your perspective? (mainly to confirm what would be a reasonable priority for investigating this bug so that it matches the priority of the work blocked on it).

Comment 2

[William Durand [:willdurand]]

I think the problem is that when we change the theme in the colorways closest (Soft/Balanced/Bold buttons), we disable/enable the current theme, which makes about:addons to "jump" (because a card gets removed/re-added).

Comment 3

[Dão Gottwald [:dao]]

(In reply to William Durand [:willdurand] from comment #2)

I think the problem is that when we change the theme in the colorways closest (Soft/Balanced/Bold buttons), we disable/enable the current theme, which makes about:addons to "jump" (because a card gets removed/re-added).

That's correct but also intentional. I think the fix here would be to make it so that we skip displaying the split second where things seem to move up (?) before being pushed down again. Not sure what exactly is happening there in the about:addons code, but it seems like something we should be able to optimize away.

(In reply to Luca Greco [:rpl] [:luca] [:lgreco] from comment #1)

Hi :dao,
may I ask you what would be the priority of this issue from your perspective? (mainly to confirm what would be a reasonable priority for investigating this bug so that it matches the priority of the work blocked on it).

This seems like a minor polish issue. I don't think we would block on this.

Comment 4

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1779714 - Defer re-rendering about:addons themes list view when active theme changes while ColorwayCloset modal dialog is open.

This patch includes a possible approach to prevent the flickering issue tracked by this bug, due to the
themes list view reacting to the theme addons changes triggered by the ColorwayCloset modal dialog
(and also take into account another similar flickering issue triggered by the changes included in this
patch without having special handled the case where the active theme is reverted automatically when
the colorway closet modal dialog is being closet and the current colorway theme is the same that
was originally active).

Comment 5

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Dão Gottwald [::dao] from comment #3)

(In reply to William Durand [:willdurand] from comment #2)

I think the problem is that when we change the theme in the colorways closest (Soft/Balanced/Bold buttons), we disable/enable the current theme, which makes about:addons to "jump" (because a card gets removed/re-added).

Yep, the issue is definitely triggered by the fact that the themes are being changed from inside the colorways closet dialog and that triggers the list view to be reloaded.

The custom element that manages the addon list does defer updating the list in a few cases, e.g. when the card is focused or its menu opened, to avoid shaking the page content while the user is actively using the element that would need to be moved elsewhere (e.g. from the enabled to the disabled section).

We could use a similar strategy to prevent the flickering that can be seen in comment 0 screencast.

(In reply to Luca Greco [:rpl] [:luca] [:lgreco] from comment #1)

Hi :dao,
may I ask you what would be the priority of this issue from your perspective? (mainly to confirm what would be a reasonable priority for investigating this bug so that it matches the priority of the work blocked on it).

This seems like a minor polish issue. I don't think we would block on this.

Sounds good, in the meantime in comment 4 I've attached a patch to provide some pointers about how we may prevent the flickering issue by deferring re-rendering the addon list until the colorways closet dialog get closed, so that the engineer that will work on that piece can use it as a starting point to prepare a fix for the issue once we got to the phase where that would become a blocking issue.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c59f026d4b26
Defer re-rendering about:addons themes list view when active theme changes while ColorwayCloset modal dialog is open. r=rpl,dao

Comment 7

[Sandor Molnar[:smolnar]]

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

Comment 8

[Alex Cornestean]

Verified as Fixed on the latest Nightly (109.0a1/20221116182402) and Beta (108.0b2/20221115200658). Tested on Windows 10 x64 and Ubuntu 16.04 LTS.

Clicking through Soft/Balanced/Bold in the Colorway modal no longer causes flickering of the about:addons theme page, confirming the fix.

See the attached video for more details. The attached video depicts the switching between the Soft/Balanced/Bold in the Colorway modal on Nightly, but the same results are observed on Beta.

Comment 9

[Alex Cornestean]

Attachment: 2022-11-17_10h00_33.mp4