1302571 - Tab border detached from tab body with LWT

Status: NEW

(Core :: Layout, defect, P3)

Description

[Justin Dolske [:Dolske]]

Attachment: Screenshot

The right-hand side of a the tab's border is detached from / has a gap with the main body of the tab when using a LWT. The left-side is fine.

I vaguely remember an old bug similar to this, but thought we fixed it. Can't find any current bugs.

Comment 1

[:Gijs (out for now; he/him)]

(In reply to Justin Dolske [:Dolske] from comment #0)
> Created attachment 8790995 [details]
> Screenshot
> 
> The right-hand side of a the tab's border is detached from / has a gap with
> the main body of the tab when using a LWT. The left-side is fine.

On https://addons.mozilla.org/en-US/firefox/addon/groovy-blue/?src=ss (when actually installed, but somehow not when previewing?) I see overlap *outside* of the left-hand border as well.

> I vaguely remember an old bug similar to this, but thought we fixed it.
> Can't find any current bugs.

Looking through mozregression.

Comment 2

[:Gijs (out for now; he/him)]

(In reply to :Gijs Kruitbosch from comment #1)
> (In reply to Justin Dolske [:Dolske] from comment #0)
> > Created attachment 8790995 [details]
> > Screenshot
> > 
> > The right-hand side of a the tab's border is detached from / has a gap with
> > the main body of the tab when using a LWT. The left-side is fine.
> 
> On https://addons.mozilla.org/en-US/firefox/addon/groovy-blue/?src=ss (when
> actually installed, but somehow not when previewing?) I see overlap
> *outside* of the left-hand border as well.

For whatever reason I can't really reproduce this anymore - it happens when the theme is not yet installed, but goes away afterwards.


I see minor gaps in the "fill" inside the tab border on both sides (symmetrically) when the theme is fully installed, but not the wide gap in your screenshot.

What OS, build and lwtheme is this? On Windows 10 I don't see lwtheme borders on tabs at all anymore.

Comment 3

[Justin Dolske [:Dolske]]

Attachment: Screenshot 2

> > The right-hand side of a the tab's border is detached from / has a gap with
> > the main body of the tab when using a LWT. The left-side is fine.
> 
> On https://addons.mozilla.org/en-US/firefox/addon/groovy-blue/?src=ss (when
> actually installed, but somehow not when previewing?) I see overlap
> *outside* of the left-hand border as well.

Wow, that's weird. I the the left-hand glitch too (when previewing), after  I installed it ans switched tabs it seemed to be mostly normal? I wonder if this is a gfx/svg glitch?

I'm on a current Nightly, OS X.

Comment 4

[Justin Dolske [:Dolske]]

Attachment: Screenshot 3

Even weirder: The active tab in the background window (top) is totally wonky, whereas the active tab in the foreground window (bottom) is almost normal. (There's still a slight gap, but it's hard to notice without zooming in.)

Comment 5

[Justin Dolske [:Dolske]]

Testing with https://addons.mozilla.org/en-US/firefox/addon/groovy-blue/?src=ss and treating the "slight gap" as ok (I think that's existed since Australis)...

Mozregression got me down to this range on Nightly: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5f16c6c2b969f70e8da10ee34853246d593af412&tochange=0010c0cb259e28faf764949df54687e3a21a2d0a

At this point mozregression seemed to get confused, it was stepping into the B2G merges and I got console spew about "Unable to find build info using the taskcluster route" / "Expanding [lower|higher] limit of the range"... It seemed to keep looping over the same builds (which were all good) so I killed it.

Nothing glaringly obvious in that range, but there are some CSS and layout changes:

bug 1069192, bug 1032613, bug 1126230, bug 1207378.

The second one (bug 1032613) mentions SVG, so if I had to choose one... dholbert, does this seem like it might be yours?

Comment 6

[Daniel Holbert [:dholbert]]

Yeah, that sounds like the sort of bug that might have caused a glitch like this. (its "part 2" patch specifically)

That patch reduced the number of frames that we use when updating overflow areas (which are the areas that get invalidated/repainted, when a repaint is needed).  That might explain why this manifests as a temporary glitch & (sometimes?) goes away after a tab-switch (per comment 3).

There must be some frame that was previously getting tracked, which is no longer tracked as a result of that patch, and so now we're repainting a smaller area.  (though we don't necessarily want to just get that frame added back -- it might be that it was just masking a different painting-bug, by getting us to repaint an area that we already should've been repainting for other reasons.)

Do you have concrete STR?  Have you seen this on non-Mac? I just tested on linux (fresh profile, apply groovy-blue lightweight theme) and I couldn't reproduce anything like the glitches shown in the screenshots.

Comment 7

[Daniel Holbert [:dholbert]]

(ni=dolske for questions in comment 6)
(tentatively marking this is a regression from bug 1032613 -- though as I noted in comment 6, it's possible (perhaps likely) that bug 1032613 just unmasked a latent invalidation/repaint bug)

Comment 8

[Justin Dolske [:Dolske]]

My STR was pretty basic. From a new profile, load https://addons.mozilla.org/en-US/firefox/addon/groovy-blue/?src=ss, hover/click the Add To Firefox button. I'm on OS X, Retina.

Comment 9

[Milan Sreckovic [:milan] (needinfo for best results)]

Daniel, what should we do with this one?  Can we fix it in 51?

Comment 10

[Daniel Holbert [:dholbert]]

I'm not sure, since I haven't been able to reproduce (on Linux).   I suspect it's retina-dependent... I'm going to get a loaner retina-mac from IT and see what I can find out.

Comment 11

[Daniel Holbert [:dholbert]]

I can reproduce this on a loaner mac. With targeted builds, I determined this regressed from bug 1194480, *not* from bug 1032613.

That confirms that this was a latent rendering bug, and we were just getting saved from exposing that bug via an unnecessary/extra reflow caused by "text-shadow" and/or "box-shadow" changes.  (If we didn't apply those properties for lightweight themes, this issue would've been visible before that commit, presumably.)

Comment 12

[Daniel Holbert [:dholbert]]

OK, I can reproduce this on my main Lenovo laptop, too, but only on Windows (Win10) -- not Linux. It's subtle on Windows -- rather than a "detached gap" at the tab border, I instead see a "sharp corner" at the upper-left edge of the tab.  And just like on Mac, the glitch goes away on tab-switch.)

I don't have cycles to look into this thoroughly today, but I'll dive into it more next week, I think.

(In reply to Milan Sreckovic [:milan] from comment #9)
> Can we fix it in 51?

(Maybe, depending on how complex the fix ends up being.   Given that we've been shipping this since Firefox 44, and given that it's varying degrees of subtle, & only reproduces briefly from infrequently-performed STR: I'd say there's probably no huge urgency to get a fix out & backported ASAP. Though it'd definitely be nice to fix.)

Comment 13

[Daniel Holbert [:dholbert]]

Attachment: hackaround (expensive, not an actual fix)

Just for demonstration/testing purposes, here's a 1-liner hackaround [which we should not actually ship].

I tested this yesterday -- it basically restores the (expensive/should-be-unnecessary) reflow on text-shadow changes that we were doing before bug 1194480.

Comment 14

[Astley Chen (inactive)]

(In reply to Daniel Holbert [:dholbert] from comment #12)
> (In reply to Milan Sreckovic [:milan] from comment #9)
> (Maybe, depending on how complex the fix ends up being.   Given that we've
> been shipping this since Firefox 44, and given that it's varying degrees of
> subtle, & only reproduces briefly from infrequently-performed STR: I'd say
> there's probably no huge urgency to get a fix out & backported ASAP. Though
> it'd definitely be nice to fix.)

So, looks like it's a carry-over regression instead of new regression here. Can we put it clear on status flag ?

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Do the steps to reproduce (STR) always involve installing/activating a lightweight theme (LWT)?  Or are there some other steps?

Comment 16

[Daniel Holbert [:dholbert]]

(In reply to Astley Chen [:astley] (UTC+8) from comment #14)
> So, looks like it's a carry-over regression instead of new regression here.
> Can we put it clear on status flag ?

I've now set firefox49 and 50 as "affected". (Doesn't look like Bugzilla's UI lets me edit the status of older branches.)

(In reply to David Baron :dbaron: ⌚️UTC-7 from comment #15)
> Do the steps to reproduce (STR) always involve installing/activating a
> lightweight theme (LWT)?  Or are there some other steps?

Those are the only steps we have at the moment, yeah. (And they only work on Mac & Windows.)

Presumably we could come up with a reduced testcase that would demonstrate the same issue, but we haven't yet.

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Reflecting data from comment 12 with the magic of "Edit multiple bugs at once" (which shows up on a query returning more than one bug), but which can be used to edit only one of them.

Comment 18

[Liz Henry (:lizzard) (relman/hg->git project)]

Too late to fix in 49.

Comment 19

[Andrew Overholt [:overholt]]

Given the fact that this isn't new I'm going to make this fix-optional for 50.

Comment 20

[Daniel Holbert [:dholbert]]

Is this still reproducible (in the tabstrip), with the Photon retheming?

Comment 21

[Justin Dolske [:Dolske]]

This no longer applies to Firefox 57, since curvy tabs are dead. (Viva la square tabs!)

I don't know if the issue here also would have applied to content in some circumstances, though.