1151259 - white thick border on button bar on linux developer tools inspector

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[jorrete]

Attachment: dark_theme_border.png

User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:37.0) Gecko/20100101 Firefox/37.0
Build ID: 20150327124350

Steps to reproduce:

Open the developer tools inspector


Actual results:

there is white border on top and bottom (3px aprox) of the button bar


Expected results:

no white border on the dark them and also the border is too thick

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

I can confirm this happens in Nightly (40) on Linux.

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

The bug appears to be present in 38 - 40, but 37 appears correctly.

It looks like <toolbar> has a default style of padding: 2px 0[1] on Linux only which leads to this.  However, that styling has been present for many years, so it can't be the root cause.

Brian, any guess what might have introduced this?

[1]: https://dxr.mozilla.org/mozilla-central/source/toolkit/themes/linux/global/toolbar.css#23

Comment 3

[Brian Grinstead [:bgrins]]

Got a regression range of: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=2b6bd6b2dfb6&tochange=369a8f14ccf8.

Most likely candidate seems to be https://hg.mozilla.org/integration/mozilla-inbound/rev/0d7bbd6633c3 (Bug 1101569), which changed the way those sidebar tabs are generated.

The other changeset that landed in that bug was includes the line that introduces the toolbar: https://hg.mozilla.org/mozilla-central/rev/e40dfb7e4ec7#l1.131.

Comment 4

[Brian Grinstead [:bgrins]]

Here is the toolbar element: https://dxr.mozilla.org/mozilla-central/source/browser/devtools/framework/sidebar.js#123.  I don't see any styles that are intended to apply to this.  If that's the case, the easiest fix would be to just switch that to be a box element.  Patrick, any reason in particular this is a toolbar or can it be switched?

Comment 5

[Patrick Brosset <:pbro>]

(In reply to Brian Grinstead [:bgrins] from comment #4)
> Here is the toolbar element:
> https://dxr.mozilla.org/mozilla-central/source/browser/devtools/framework/
> sidebar.js#123.  I don't see any styles that are intended to apply to this. 
> If that's the case, the easiest fix would be to just switch that to be a box
> element.  Patrick, any reason in particular this is a toolbar or can it be
> switched?
As long as the <toolbarbutton> in that toolbar still works, this can be anything at all.

Comment 6

[Brian Grinstead [:bgrins]]

Attachment: tabs-container.patch

Does this do the trick?

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4463030d9ed3

Comment 7

[Brian Grinstead [:bgrins]]

Comment on attachment 8589111 [details] [diff] [review]
tabs-container.patch

Looks like there are some test failures due to the element changing

Comment 8

[Brian Grinstead [:bgrins]]

Attachment: tabs-container.patch

This is looking better: https://treeherder.mozilla.org/#/jobs?repo=try&revision=03444007ee74

Comment 9

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8589304 [details] [diff] [review]
tabs-container.patch

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

Great, works well here!

Comment 10

[Brian Grinstead [:bgrins]]

https://hg.mozilla.org/integration/fx-team/rev/77303df92deb

Comment 11

[[Back August 16] Ryan VanderMeulen]

https://hg.mozilla.org/mozilla-central/rev/77303df92deb

Comment 12

[Brian Grinstead [:bgrins]]

Comment on attachment 8589304 [details] [diff] [review]
tabs-container.patch

Approval Request Comment
[Feature/regressing bug #]: 1101569
[User impact if declined]: On Linux there is an ugly border in the Inspector and Netmonitor panels in devtools
[Describe test coverage new/current, TreeHerder]: None, markup change only
[Risks and why]: Low risk, changing a <toolbar> element to a <box> in the sidebar.
[String/UUID change made/needed]:

Comment 13

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8589304 [details] [diff] [review]
tabs-container.patch

Should be in 38 beta 5.

Comment 14

[[Back August 16] Ryan VanderMeulen]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4de4a3aefc0a

Comment 15

[[Back August 16] Ryan VanderMeulen]

https://hg.mozilla.org/releases/mozilla-beta/rev/7af104b169fa