624679 - Border between nav bar and content should be opaque

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Dão Gottwald [:dao]]

Attachment: patch

Comment 1

[Frank Yan (:fryn)]

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

>+  #main-window:not([disablechrome]) #navigator-toolbox[tabsontop="true"] {
>+    border-bottom-left-radius: 2px;
>+    border-bottom-right-radius: 2px;
>+  }

This is a clever way to clip the border such that it doesn't extend all the way to the left and right edges, but when comparing the mockup, it still doesn't match up, since the vertical border is supposed to be continuous from the navigation bar to the content area. If we want to match the mockup's border, we should be applying border-bottom on a child element of #navigator-toolbox, not #navigator-toolbox itself.

Also, by applying a border-radius, the last two pixels at each edge are partially transparent, which leaves that half of bug 617506 still unresolved. If this bug is just supposed to help fix bug 617506, since it blocks the release, why can't we simply remove/fill in the glass borders of #navigator-toolbox to fix the blocker and then leave the part of making it look more like the mockups to a followup?

Comment 2

[Frank Yan (:fryn)]

Attachment: screenshot with patch applied

Comment 3

[Frank Yan (:fryn)]

Stephen, I think you mentioned this on IRC, but I've forgotten: what were your thoughts on how this could be fixed, if even just a temporary fix that we could ship? (since roc wants the transparent/semi-transparent pixels removed if they have nothing opaque underneath)

Comment 4

[Dão Gottwald [:dao]]

(In reply to comment #1)
> This is a clever way to clip the border such that it doesn't extend all the way
> to the left and right edges, but when comparing the mockup, it still doesn't
> match up, since the vertical border is supposed to be continuous from the
> navigation bar to the content area. If we want to match the mockup's border, we
> should be applying border-bottom on a child element of #navigator-toolbox, not
> #navigator-toolbox itself.

We don't know what child element that would be, since there can be extension toolbars, and collapsed toolbars.

> Also, by applying a border-radius, the last two pixels at each edge are
> partially transparent, which leaves that half of bug 617506 still unresolved.
> If this bug is just supposed to help fix bug 617506, since it blocks the

It's supposed to improve the looks, I don't think we're blocking on bug 617506 comment 2.

> release, why can't we simply remove/fill in the glass borders of
> #navigator-toolbox to fix the blocker and then leave the part of making it look
> more like the mockups to a followup?

I don't know how that followup would look like.

Comment 5

[Frank Yan (:fryn)]

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

Pondered it some more; I think this satisfactorily fixes the bug description.

Could use a nod from Stephen.

Comment 6

[Justin Dolske [:Dolske]]

Happen to have a not-zoomed in screenshot? The 1px gap on the vertical border seems like it might be distracting.

Comment 7

[Frank Yan (:fryn)]

(In reply to bug 617506 comment #25)
> http://hg.mozilla.org/mozilla-central/rev/a6eb68c190c2
> 
> bug 624679 handles the border above the content area

With the current patch applied, the border above the content area still disappears on about:addons.

(In reply to comment #6)
> Happen to have a not-zoomed in screenshot? The 1px gap on the vertical border
> seems like it might be distracting.

I don't happen to have one, but I did not find it especially distracting.

A pixel-perfect solution is to make the toolbox include a 1px tall element for the horizontal border between the toolbars and the content; the vertical border could then be extended across the currently odd 1px gap.

Comment 8

[Dão Gottwald [:dao]]

Attachment: screenshot

Comment 9

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/9136f40c8f05

filed bug 627324 on the non-continuous shadow