980248 - DevTools Themes: Extra top border showing up on responsive-container splitters when width > 700px

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Patrick Brosset <:pbro>]

This is a screenshot of a portion of the inspector:
https://dl.dropboxusercontent.com/u/714210/Screenshot%202014-03-06%2011.14.40.png

If you look at the top of the splitter element (between the markup-view and rule-view), you will see that there are a couple of pixels that shouldn't be there.

Here is another screenshot in which I've made the splitter blue and wider, so it's easier to see:
https://dl.dropboxusercontent.com/u/714210/Screenshot%202014-03-06%2011.14.23.png

This is really minor, but some people (like me) might notice it.
I've quickly tried to play with margin/padding/border/... on the splitter element but no css property I tried to reset got rid of the extra pixels.

Comment 1

[Patrick Brosset <:pbro>]

To be really clear, here is what I'm talking about:
https://dl.dropboxusercontent.com/u/714210/Screenshot%202014-03-06%2011.14.40%20copy.png

Comment 2

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

Same happens in the debugger.

Comment 3

[Brian Grinstead [:bgrins]]

Attachment: splitter-extra-border.patch

There was an additional top border being added in all cases when the splitter was inside a .devtools-responsive-container.  It should only be added when width < 700px.

Rather than just wrap the rule in a media query, I've reorganized it a bit to avoid having 3 magic numbers for responsive width.

Comment 4

[Patrick Brosset <:pbro>]

Comment on attachment 8386794 [details] [diff] [review]
splitter-extra-border.patch

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

Tested, works fine.
Had a little trouble understanding comment 3 at first, but got it now.

Comment 5

[Brian Grinstead [:bgrins]]

https://hg.mozilla.org/integration/fx-team/rev/bb950168dbda
https://tbpl.mozilla.org/?tree=Fx-Team&rev=bb950168dbda

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/bb950168dbda

Comment 7

[Carol Chung[:lizardwalk5]]

[bugday-20140409]

Hi, I'm getting a 404 error when I try to view the dropbox screenshots (in bug description and comment 1). Could I get a screenshot to check which part of the Inspector splitter to look at?

Is it a valid check if I compare the splitter between Aurora and Firefox 28?

Comment 8

[Patrick Brosset <:pbro>]

You should rather compare the splitter between version 29 (current release) and aurora (30), since this bug got fixed in 30. 
The bug doesn't exist in 28 since it came along with the huge devtools theme changes made in 29.

Having said that, I'm trying to replicate the bug in 29, but I'm not able to for some reasons.

Brian, do you remember how to replicate this issue so it can be verified?

Comment 9

[Brian Grinstead [:bgrins]]

(In reply to Patrick Brosset [:pbrosset] [:patrick] from comment #8)
> You should rather compare the splitter between version 29 (current release)
> and aurora (30), since this bug got fixed in 30. 

Correction: version 29 is currently the beta release.

> The bug doesn't exist in 28 since it came along with the huge devtools theme
> changes made in 29.
> 
> Having said that, I'm trying to replicate the bug in 29, but I'm not able to
> for some reasons.
> 
> Brian, do you remember how to replicate this issue so it can be verified?

This was a minor issue in which the splitter bled over a pixel or two and I think maybe covered up a pixel on the selected sidebar tab in the inspector.  But I'm looking at 29 on Beta and cannot see the issue.  The patch is not applied in this environment by checking chrome://browser/skin/devtools/light-theme.css and chrome://browser/skin/devtools/common.css.

It *was* a really minor issue, so maybe I'm just not seeing it right now.  The screenshot that was in Comment 1 had an arrow pointing to the exact problem if there is some way to recover it.

Comment 10

[Brian Grinstead [:bgrins]]

Attachment: same-splitters.png

I've looked further into this - comparing the splitter in 29 vs 30 and I can't see any difference.  The only thing I can think is that a separate bug that got uplifted to 29 has fixed or masked this problem, even though this patch is only on 30.