1000704 - The border between the toolbox and customize palette has a border which leaks into the padding

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Matthew N. [:MattN]]

Attachment: releases-mozilla-beta-b074e386a410_102_tabsOutsideTitlebar_fourPinned_maximized_allToolbars_noLWT_customizing.png

The border extends a few pixels into the padding around cust. mode.

Comment 1

[Mike de Boer [:mikedeboer]]

Attachment: Patch v1: don't display the navigator-toolbox bottom border in customize mode

Matt, I added an attribute setter for gNavToolbox, because I didn't want to add another descendant selector that might regress a Talos suite.

Comment 2

[Matthew N. [:MattN]]

Comment on attachment 8411665 [details] [diff] [review]
Patch v1: don't display the navigator-toolbox bottom border in customize mode

Review of attachment 8411665 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/themes/osx/browser.css
@@ +21,5 @@
>  .searchbar-textbox:-moz-lwtheme:not([focused="true"]) {
>    opacity: .9;
>  }
>  
> +#navigator-toolbox:not([customizing])::after {

The problem isn't that the border is present, it's that it extends too far. I don't think we want to remove the bottom border as that's just trading one bug for arguably a bigger one: http://grab.by/wliw

Comment 3

[Mike de Boer [:mikedeboer]]

Attachment: Patch v2: don't display the navigator-toolbox bottom border in customize mode

You're completely right, of course.

What about this version?

Comment 4

[Matthew N. [:MattN]]

Comment on attachment 8412635 [details] [diff] [review]
Patch v2: don't display the navigator-toolbox bottom border in customize mode

Review of attachment 8412635 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/themes/osx/browser.css
@@ +33,5 @@
>    z-index: 2; /* navbar is at 1 */
>  }
>  
> +#navigator-toolbox[customizing]::after {
> +  margin-left: 3px;

Well, where did these numbers come from? Trial and error?

Comment 5

[Mike de Boer [:mikedeboer]]

Comment on attachment 8412635 [details] [diff] [review]
Patch v2: don't display the navigator-toolbox bottom border in customize mode

On second thought, I don't like adding JS to make the selector prettier.

I'll up a patch without JS and we'll see if it affects cart perf.

Comment 6

[Mike de Boer [:mikedeboer]]

(In reply to Matthew N. [:MattN] from comment #4)
> Well, where did these numbers come from? Trial and error?

Nono! I wouldn't dare ;) http://mxr.mozilla.org/mozilla-central/source/browser/themes/osx/browser.css#4292

Comment 7

[Mike de Boer [:mikedeboer]]

Attachment: Patch v3: don't let the navigator-toolbox border bleed through in customization mode

Comment 8

[Matthew N. [:MattN]]

Comment on attachment 8412851 [details] [diff] [review]
Patch v3: don't let the navigator-toolbox border bleed through in customization mode

Review of attachment 8412851 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with or without the change (if you disagree with my suggestion).

::: browser/themes/osx/browser.css
@@ +4295,5 @@
>  #main-window[customize-entered] #navigator-toolbox > toolbar:not(#TabsToolbar) {
>    border-bottom-width: 0;
>  }
>  
>  #main-window[customize-entered] #TabsToolbar {

Did you consider just putting the new rule "#main-window[customize-entered] #navigator-toolbox::after" above this one?

Comment 9

[Mike de Boer [:mikedeboer]]

I pushed it to fx-team with your suggestion: https://hg.mozilla.org/integration/fx-team/rev/096b4bc2a9ae

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/096b4bc2a9ae