1406735 - Right border of selected pinned tab looks wrong with overflowing tab strip

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[Dão Gottwald [:dao]]

Attachment: screenshot

I see this on Ubuntu but it's probably cross-platform.

Comment 1

[Dão Gottwald [:dao]]

Attachment: Bug 1406735 - Prevent pinned tab separator from overlapping the tab background.

Review commit: https://reviewboard.mozilla.org/r/188800/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/188800/

Comment 2

[Johann Hofmann [:johannh]]

Comment on attachment 8917858 [details]
Bug 1406735 - Prevent pinned tab separator from overlapping the tab background.

https://reviewboard.mozilla.org/r/188800/#review194112

Works for me, with the caveat that there are very slight rendering issues as mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1406691#c9, but I'm willing to make that trade.

Comment 3

[Dão Gottwald [:dao]]

(In reply to Johann Hofmann [:johannh] from comment #2)
> Comment on attachment 8917858 [details]
> Bug 1406735 - Prevent pinned tab separator from overlapping the tab
> background.
> 
> https://reviewboard.mozilla.org/r/188800/#review194112
> 
> Works for me, with the caveat that there are very slight rendering issues as
> mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1406691#c9, but
> I'm willing to make that trade.

Are you assuming that or do you actually see the issue with this patch?

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b4c54a5506af
Prevent pinned tab separator from overlapping the tab background. r=johannh

Comment 5

[Johann Hofmann [:johannh]]

(In reply to Dão Gottwald [::dao] from comment #3)
> (In reply to Johann Hofmann [:johannh] from comment #2)
> > Comment on attachment 8917858 [details]
> > Bug 1406735 - Prevent pinned tab separator from overlapping the tab
> > background.
> > 
> > https://reviewboard.mozilla.org/r/188800/#review194112
> > 
> > Works for me, with the caveat that there are very slight rendering issues as
> > mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1406691#c9, but
> > I'm willing to make that trade.
> 
> Are you assuming that or do you actually see the issue with this patch?

I assumed it initially, because it was using the same code from bug 1406691. Trying it out now, I have to say it's quite hard to reproduce, for me at least. From all the DPI settings on my computer it only appears on 150% and there only on every ~4th pinned tab.

Comment 6

[Dão Gottwald [:dao]]

(In reply to Johann Hofmann [:johannh] from comment #5)
> (In reply to Dão Gottwald [::dao] from comment #3)
> > (In reply to Johann Hofmann [:johannh] from comment #2)
> > > Comment on attachment 8917858 [details]
> > > Bug 1406735 - Prevent pinned tab separator from overlapping the tab
> > > background.
> > > 
> > > https://reviewboard.mozilla.org/r/188800/#review194112
> > > 
> > > Works for me, with the caveat that there are very slight rendering issues as
> > > mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1406691#c9, but
> > > I'm willing to make that trade.
> > 
> > Are you assuming that or do you actually see the issue with this patch?
> 
> I assumed it initially, because it was using the same code from bug 1406691.
> Trying it out now, I have to say it's quite hard to reproduce, for me at
> least. From all the DPI settings on my computer it only appears on 150% and
> there only on every ~4th pinned tab.

As far as I can see, you were using padding-inline-end. I'm using border-inline-end as it should avoid bug 477157.

Comment 7

[Johann Hofmann [:johannh]]

(In reply to Dão Gottwald [::dao] from comment #6)
> (In reply to Johann Hofmann [:johannh] from comment #5)
> > (In reply to Dão Gottwald [::dao] from comment #3)
> > > (In reply to Johann Hofmann [:johannh] from comment #2)
> > > > Comment on attachment 8917858 [details]
> > > > Bug 1406735 - Prevent pinned tab separator from overlapping the tab
> > > > background.
> > > > 
> > > > https://reviewboard.mozilla.org/r/188800/#review194112
> > > > 
> > > > Works for me, with the caveat that there are very slight rendering issues as
> > > > mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1406691#c9, but
> > > > I'm willing to make that trade.
> > > 
> > > Are you assuming that or do you actually see the issue with this patch?
> > 
> > I assumed it initially, because it was using the same code from bug 1406691.
> > Trying it out now, I have to say it's quite hard to reproduce, for me at
> > least. From all the DPI settings on my computer it only appears on 150% and
> > there only on every ~4th pinned tab.
> 
> As far as I can see, you were using padding-inline-end. I'm using
> border-inline-end as it should avoid bug 477157.

Ah, indeed, great! That explains why it's harder to reproduce now :D

I'm leaning towards discarding the exotic rendering issue I've dug up then, since I don't think there's much we can do at this point and it's unlikely to get noticed by anyone who's not trying to break things on purpose.

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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

Comment 9

[ovidiu boca[:Ovidiu]]

I verified this on Ubuntu 16.04 and Mac Os X 10.10 with FF Nightly 58.0a1(2017-10-15) and I can't reproduce the issue presented in the description. I will mark this as verified fixed.