830684 - Sidebar toolbox height is 1px higher than the breadcrumbs on mac

Status: RESOLVED FIXED

(DevTools :: Framework, defect, P2)

Comment 1

[Paul Rouget [:paul]]

See bug 829063 comment 5

Comment 2

[Paul Rouget [:paul]]

What version of osx do you have?

Comment 3

[Victor Porof [:vporof][:vp]]

(In reply to Paul Rouget [:paul] from comment #2)
> What version of osx do you have?

I've seen this happening sometimes on my machine (Mountain Lion), but not always.

Comment 4

[Heather Arthur [:harth]]

I have 10.8 and a Retina.

Comment 5

[Paul Rouget [:paul]]

Heather, can you attach a screenshot please?

Comment 6

[Heather Arthur [:harth]]

Attachment: screenshot of the 1px difference

Here's a screenshot of it.

Comment 7

[Paul Rouget [:paul]]

(In reply to Heather Arthur [:harth] from comment #6)
> Created attachment 705210 [details]
> screenshot of the 1px difference
> 
> Here's a screenshot of it.

Can I ask to take a screenshot of the whole toolbox (I want to compare the font size).

Comment 8

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Fedora Screenshot

Far worse on Fedora (Gnome 3)

Comment 9

[u443210]

Attachment: Issue present on XP

Also present on XP.

Comment 10

[Victor Porof [:vporof][:vp]]

It seems that toolbar height vs. sidebar tabs height is completely and randomly different on each platform.

Comment 11

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

I suspect that it is related to font-size / line heights.

Comment 12

[Paul Rouget [:paul]]

Attachment: Patch v1

This should make the whole thing font-size agnostic (I needed to replicated the min-height of the button in the breadcrumbs).

Not tested on Windows and mac.

Comment 13

[Paul Rouget [:paul]]

Attachment: Patch v1 (fixed typo)

Comment 14

[Victor Porof [:vporof][:vp]]

Comment on attachment 770768 [details] [diff] [review]
Patch v1 (fixed typo)

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

I don't have any reason to not r+ this, but I'm little worried that this will not definitively fix the problem. See my questions below.

::: browser/themes/linux/devtools/common.css
@@ +281,4 @@
>    -moz-padding-start: 6px;
>    margin: 0;
>    min-width: 78px;
> +  min-height: 32px;

Is the sister container (the toolbar with breadcrumbs in the inspector and debugger) always guaranteed to be higher than 32px? I'm trying to understand why you added a min-height here.

Also, what happens if the toolbar doesn't contain a button? In that case, it doesn't sound like its height will match the sidebar tabs height (this used to be the case with the network monitor for a while).

Comment 15

[Paul Rouget [:paul]]

The other toolbars have a guaranteed min-height.

> Also, what happens if the toolbar doesn't contain a button?

I don't know.

Comment 16

[Victor Porof [:vporof][:vp]]

(In reply to Paul Rouget [:paul] from comment #15)
> The other toolbars have a guaranteed min-height.
> 

How? Might want to explain that in the CSS.

Comment 17

[Paul Rouget [:paul]]

Attachment: Patch v1.1 (to land)

+comments

Comment 18

[Victor Porof [:vporof][:vp]]

https://hg.mozilla.org/integration/fx-team/rev/8dee4d00c31b

Comment 19

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/8dee4d00c31b

Comment 20

[Girish Sharma [:Optimizer]]

Comment on attachment 770768 [details] [diff] [review]
Patch v1 (fixed typo)

patch is already in production.