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

RESOLVED FIXED in Firefox 32

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: MattN, Assigned: mikedeboer)

Tracking

(Blocks 1 bug, {polish})

29 Branch
Firefox 32
All
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox29 affected, firefox30 affected, firefox31 affected, firefox32 affected)

Details

(Whiteboard: [Australis:P5][mentor=MattN])

Attachments

(2 attachments, 2 obsolete attachments)

Assignee

Updated

5 years ago
Assignee: nobody → mdeboer
Status: NEW → ASSIGNED
Assignee

Comment 1

5 years ago
Matt, I added an attribute setter for gNavToolbox, because I didn't want to add another descendant selector that might regress a Talos suite.
Attachment #8411665 - Flags: review?(MattN+bmo)
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
Attachment #8411665 - Flags: review?(MattN+bmo) → review-
Assignee

Comment 3

5 years ago
You're completely right, of course.

What about this version?
Attachment #8411665 - Attachment is obsolete: true
Attachment #8412635 - Flags: review?(MattN+bmo)
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?
Assignee

Comment 5

5 years ago
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.
Attachment #8412635 - Flags: review?(MattN+bmo)
Assignee

Comment 6

5 years ago
(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 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?
Attachment #8412851 - Flags: review?(MattN+bmo) → review+
Assignee

Comment 9

5 years ago
I pushed it to fx-team with your suggestion: https://hg.mozilla.org/integration/fx-team/rev/096b4bc2a9ae
Whiteboard: [Australis:P5][mentor=MattN] → [Australis:P5][mentor=MattN][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/096b4bc2a9ae
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P5][mentor=MattN][fixed-in-fx-team] → [Australis:P5][mentor=MattN]
Target Milestone: --- → Firefox 32
You need to log in before you can comment on or make changes to this bug.