1395085 - Theme preview is glitchy on compact themes

Status: RESOLVED WORKSFORME

(Firefox :: Theme, defect)

Description

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

[Affected versions]:
- 56.0b7 (20170828185355)
- 55.0.3 (20170824053622)
- devedition 56.0b7 (20170828185355)

[Affected platforms]:
- Windows 10 x64
- Ubuntu 14.04 x86
- Mac OS X 10.11.6

[Steps to reproduce]:
1. Launch Firefox
2. Go to about:addons, select the Appearance tab and enable a compact theme (Compact Dark or Compact Light) - if it isn't already enabled
3. Navigate to https://addons.mozilla.org/en-US/firefox/themes/?src=firefox and hover the (top side of the) listed themes samples in order to preview each selected theme
- inspect the theme preview process

[Expected result]:
- Each hovered theme is properly previewed and no visual artifacts are triggered

[Actual result]:
- The preview process is glitchy (see the screencast https://goo.gl/RsvHax)

[Regression range]:
- Last good revision: 16e30af059b7 (2015-07-15)
- First bad revision: 72835344333f (2015-07-16)
- Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=16e30af059b7&tochange=72835344333f

[Additional notes]:
- The regression is not a recent one as the issue is reproducible before Dev Edition theme(s) were changed to the compact ones

Comment 1

[Panos Astithas (he/him) [:past] (please ni?)]

I don't see this in 56.0b8 on macOS, it might be fixed in the latest beta. Could you please test again?

Comment 2

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

(In reply to Panos Astithas [:past] (56 Regression Engineering Owner) (please ni?) from comment #1)
> I don't see this in 56.0b8 on macOS, it might be fixed in the latest beta.
> Could you please test again?

Tested again and reproduced the same issue on 56.0b9 build1 (20170903140023), using  Windows 10 x64, Ubuntu 14.04 x64 and Mac OS X 10.11.6.

Comment 3

[Panos Astithas (he/him) [:past] (please ni?)]

Hmm, it seems b9 isn't available yet for me to test, but one difference is that I am using macOS 10.12.6.

Comment 4

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

(In reply to Panos Astithas [:past] (56 Regression Engineering Owner) (please ni?) from comment #3)
> Hmm, it seems b9 isn't available yet for me to test, but one difference is
> that I am using macOS 10.12.6.

Also reproduced on macOS 10.12.6, using 56.0b9 build1 (20170903140023).

Comment 5

[Andrew Swan [:aswan]]

Its kind of hard to see since it happens so fast, but it looks like in the video from comment 0, the "tabs in the title bar" setting is applied but previewing the theme causes the tab bar to move out of the title bar, which causes all the content on the page to move down, which makes the mouse exit the theme, which cancels the preview.  This causes the tabs to move back to the title bar which moves the content back up and the mouse is back over the theme which causes a new preview.  Then rinse and repeat...

I'm not sure what the intention here is, but it seems to me that the "tabs in the title bar" setting should be independent of the actual theme currently applied.  If its not, I don't have any great ideas for what to do about this.

Comment 6

[:Gijs (he/him)]

(In reply to Andrew Swan [:aswan] from comment #5)
> Its kind of hard to see since it happens so fast, but it looks like in the
> video from comment 0, the "tabs in the title bar" setting is applied but
> previewing the theme causes the tab bar to move out of the title bar, which
> causes all the content on the page to move down, which makes the mouse exit
> the theme, which cancels the preview.  This causes the tabs to move back to
> the title bar which moves the content back up and the mouse is back over the
> theme which causes a new preview.  Then rinse and repeat...
> 
> I'm not sure what the intention here is, but it seems to me that the "tabs
> in the title bar" setting should be independent of the actual theme
> currently applied.  If its not, I don't have any great ideas for what to do
> about this.

I'm pretty sure it's just that in a compact theme, the toolbox isn't as tall as when using a lightweight theme (which gets applied over the top of the the default theme instead of the compact theme, which is itself a (weird) lightweight theme on 56 and below) so the web content shifts, which shifts the position of the item you're hovering to preview.

In that sense, this is a dupe of bug 1237995 but with the buttons on AMO instead of the picker in customize mode, and that bug goes back to 46 so I don't think we should worry about this for 56.

If this doesn't reproduce in 57, now that compact mode has been separated out from theme state, I think we should close as WFM (which we might want to do in the bug I referenced, too). Panos, does that sound right?

Comment 7

[Panos Astithas (he/him) [:past] (please ni?)]

Thanks for the detailed analysis Gijs, I agree that it's too late to fix this just for 56.