617506 - Unexpected rows of transparent pixels in Firefox chrome

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

See attached image. There's a row of transparent pixels below inactive tabs. There's also a row of transparent pixels between the URL bar and the Web content. This seems unexpected.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: screenshot

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Apart from looking odd, this causes us to use larger Aero Glass margins internally, which is a slight performance hit makes bug 613449 worse.

Comment 3

[Dão Gottwald [:dao]]

See bug 604941 comment 4 and below

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I don't understand from that bug why it's absolutely necessary for that row of pixels to be transparent. Can't we extend the toolbar background under it somehow?

It just seems weird to me to have a single row of glass pixels there. It feels like the URL bar has broken off.

Comment 5

[[not reading bugmail]]

Dao, I'm curious, would this have anything to do with the border issue with transition into/out of Fullscreen so glass doesn't extend into the content area or other places?  Just thought I'd ask.

Comment 6

[Dão Gottwald [:dao]]

(In reply to comment #5)
> Dao, I'm curious, would this have anything to do with the border issue with
> transition into/out of Fullscreen so glass doesn't extend into the content area
> or other places?  Just thought I'd ask.

No

Comment 7

[Justin Dolske [:Dolske]]

Unowned blocker, over to Margaret.

Comment 8

[:Margaret Leibovic]

It doesn't seem like any decision about what to do has been made here. Should I try to implement roc's suggestion in comment 4?

Stephen, what are your thoughts?

Comment 9

[Frank Yan (:fryn)]

(In reply to comment #8)
> It doesn't seem like any decision about what to do has been made here. Should I
> try to implement roc's suggestion in comment 4?

It looks like the patch in bug 624679 addresses this issue too. Dão, does that patch partially or completely fix this?

Comment 10

[Dão Gottwald [:dao]]

(In reply to comment #9)
> It looks like the patch in bug 624679 addresses this issue too. Dão, does that
> patch partially or completely fix this?

Only partially. What the lower arrow in attachment 496043 [details] points at, to be exact.

Comment 11

[Asa Dotzler [:asa]]

(In reply to comment #4)
> I don't understand from that bug why it's absolutely necessary for that row of
> pixels to be transparent. Can't we extend the toolbar background under it
> somehow?
> 
> It just seems weird to me to have a single row of glass pixels there. It feels
> like the URL bar has broken off.

The dark row of pixels at the top of the toolbar is supposed to be a shadow and so it's supposed to be semi-transparent. It's the navigation toolbar casting a shadow on background tabs and any empty glass area.

The correct thing to do is to push the background tabs' baseline down a pixel so that we have the navigation toolbar shadow on top of the background tabs instead of on top of a single pixel row of glass.

Comment 12

[Stephen Horlander [:shorlander]]

(In reply to comment #11)
> (In reply to comment #4)
> > I don't understand from that bug why it's absolutely necessary for that row of
> > pixels to be transparent. Can't we extend the toolbar background under it
> > somehow?
> > 
> > It just seems weird to me to have a single row of glass pixels there. It feels
> > like the URL bar has broken off.
> 
> The dark row of pixels at the top of the toolbar is supposed to be a shadow and
> so it's supposed to be semi-transparent. It's the navigation toolbar casting a
> shadow on background tabs and any empty glass area.
> 
> The correct thing to do is to push the background tabs' baseline down a pixel
> so that we have the navigation toolbar shadow on top of the background tabs
> instead of on top of a single pixel row of glass.

What Asa said.

Comment 13

[:Margaret Leibovic]

I played around with this a bit, but I'm not sure I know exactly what should be done. I tried adding a margin-bottom: -1px; rule to .tabbrowser-tab, which pushed the tabs down, but they moved on top of the transparent border instead of behind it.

Dão or Frank, do you have any thoughts on this? I feel like you're both more familiar with the tab styling than me.

Comment 14

[Dão Gottwald [:dao]]

I can look into this.

Comment 15

[Dão Gottwald [:dao]]

Attachment: patch

Comment 16

[Dão Gottwald [:dao]]

Attachment: patch

err, typo

Comment 17

[Stephen Horlander [:shorlander]]

Comment on attachment 503847 [details] [diff] [review]
patch

Looks great! The only problem I noticed is that it creates a line underneath the selected app-tab if you have no regular tabs open.

I will attach a screenshot of the problem.

Comment 18

[Stephen Horlander [:shorlander]]

Attachment: line under apptab

Comment 19

[Frank Yan (:fryn)]

Comment on attachment 503847 [details] [diff] [review]
patch

A top border around the content area should remain on chromeless pages, e.g. about:addons.

Besides that and shorlander's comment, the patch looks good.

Comment 20

[Frank Yan (:fryn)]

Comment on attachment 503847 [details] [diff] [review]
patch

>+@media not all and (-moz-windows-compositor) {

According to dbaron in bug 625966 comment 1, we support the shorthand syntax, so this can be shortened to:

@media not (-moz-windows-compositor) {

which is less confusing (since there is a mental tendency to parse it with the wrong association).

Comment 21

[Frank Yan (:fryn)]

(In reply to comment #20)

Never mind. It turns out that I misinterpreted the spec. Bizarre spec. :\

Comment 22

[Dão Gottwald [:dao]]

Attachment: patch v2

Fixed the only-app-tabs issue. I'll spin off the disablechrome case to a separate bug. (The border is there, but as the border of the toolbox it doesn't overlay tabs.)

Comment 23

[Dão Gottwald [:dao]]

Attachment: patch v3

the fix in the previous revision allows me to get rid of some pinstripe code

Comment 24

[Justin Dolske [:Dolske]]

Comment on attachment 504103 [details] [diff] [review]
patch v3

rs=me, leaving fryn flagged in case you wanted specific feedback from him.

Comment 25

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/a6eb68c190c2

bug 624679 handles the border above the content area

Comment 26

[Javier Berezovsky]

Fix verified in Mozilla/5.0 (Windows NT 6.1; rv:2.0b11) Gecko/20100101 Firefox/4.0b11