Unexpected rows of transparent pixels in Firefox chrome

VERIFIED FIXED in Firefox 4.0b10

Status

()

defect
VERIFIED FIXED
9 years ago
6 years ago

People

(Reporter: roc, Assigned: dao)

Tracking

unspecified
Firefox 4.0b10
x86
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 final+)

Details

(Whiteboard: [softblocker][fx4-fixed-bugday])

Attachments

(2 attachments, 4 obsolete attachments)

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.
Apart from looking odd, this causes us to use larger Aero Glass margins internally, which is a slight performance hit makes bug 613449 worse.
blocking2.0: --- → ?
Assignee

Comment 3

9 years ago
See bug 604941 comment 4 and below
Blocks: 604941
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.
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.
Assignee

Comment 6

9 years ago
(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
blocking2.0: ? → final+
Assignee

Updated

9 years ago
Depends on: 624679
Unowned blocker, over to Margaret.
Assignee: nobody → margaret.leibovic
Whiteboard: [softblocker]
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

9 years ago
(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?
Assignee

Comment 10

9 years ago
(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.
(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.
(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.
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.
Assignee

Comment 14

9 years ago
I can look into this.
Assignee: margaret.leibovic → dao
Assignee

Comment 15

9 years ago
Posted patch patch (obsolete) — Splinter Review
Attachment #503846 - Flags: review?(gavin.sharp)
Attachment #503846 - Flags: feedback?(fryn)
Assignee

Comment 16

9 years ago
Posted patch patch (obsolete) — Splinter Review
err, typo
Attachment #503846 - Attachment is obsolete: true
Attachment #503847 - Flags: review?(gavin.sharp)
Attachment #503847 - Flags: feedback?(fryn)
Attachment #503846 - Flags: review?(gavin.sharp)
Attachment #503846 - Flags: feedback?(fryn)
Attachment #503847 - Flags: review?(gavin.sharp) → review?(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.
Attachment #503847 - Flags: review?(shorlander) → review-
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.
Attachment #503847 - Flags: feedback?(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).
(In reply to comment #20)

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

Comment 22

9 years ago
Posted patch patch v2 (obsolete) — Splinter Review
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.)
Attachment #503847 - Attachment is obsolete: true
Attachment #503929 - Attachment is obsolete: true
Attachment #504091 - Flags: review?(fryn)
Assignee

Comment 23

9 years ago
Posted patch patch v3Splinter Review
the fix in the previous revision allows me to get rid of some pinstripe code
Attachment #504091 - Attachment is obsolete: true
Attachment #504103 - Flags: review?(fryn)
Attachment #504091 - Flags: review?(fryn)
Comment on attachment 504103 [details] [diff] [review]
patch v3

rs=me, leaving fryn flagged in case you wanted specific feedback from him.
Attachment #504103 - Flags: review+
Assignee

Comment 25

9 years ago
http://hg.mozilla.org/mozilla-central/rev/a6eb68c190c2

bug 624679 handles the border above the content area
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b10
Assignee

Updated

9 years ago
Attachment #504103 - Flags: review?(fryn)
Assignee

Updated

9 years ago
Depends on: 626351
Fix verified in Mozilla/5.0 (Windows NT 6.1; rv:2.0b11) Gecko/20100101 Firefox/4.0b11
Status: RESOLVED → VERIFIED
Whiteboard: [softblocker] → [softblocker][fx4-fixed-bugday]
Assignee

Updated

9 years ago
Depends on: 631698
You need to log in before you can comment on or make changes to this bug.