1252640 - Fix overlapping borders in the ruleview-header

Status: RESOLVED FIXED

(DevTools :: Inspector: Rules, defect, P3)

Description

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Screenshot

We can see an overlaying border when we have a couple of situations:
(1) The pseudo-elements container is present (Enable Browser Styles)
(2) The pseudo-elements container is not expanded and the next element is a ruleview-header

Comment 1

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: 1252640.patch

Comment 2

[Gabriel Luong [:gl] (ΦωΦ)]

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

Comment 3

[Patrick Brosset <:pbro>]

Comment on attachment 8725454 [details] [diff] [review]
1252640.patch

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

So I've got one problem with this patch: today the display between MacOS and Windows is pretty different:

- On MacOS, when there are 2 collapsible sections next to each other, they touch, and so you end up with the double border problem that you're aiming at fixing here.
- On Windows however, the 2 sections are far apart (like maybe 3 or 4px apart), and so the borders don't touch.

Therefore, this patch makes it look a little bit weird on Windows I'd say.
I'll attach before/after screenshots.
I do think we should have the same display on both OS.

::: devtools/client/inspector/rules/rules.js
@@ +1097,3 @@
>     */
> +  _toggleContainerVisibility: function(twisty, container, isPseudo,
> +      showPseudo) {

super nit: please align showPseudo with twisty.
When we wrap long lines, we either indent the second line with 2 extra spaces, or try to find a good place to vertically align the second line. For functions, vertically aligning with the other arguments above is often a good thing:

_toggleContainerVisibility: function(twisty, container, isPseudo,
                                     showPseudo) {

Comment 4

[Patrick Brosset <:pbro>]

Attachment: windows-before.png

Comment 5

[Patrick Brosset <:pbro>]

Attachment: windows-after.png

Comment 6

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: 1252640.patch [1.0]

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/48199a0c1e54

Comment 8

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

I had to back this out because it appeared to cause this test failure: https://treeherder.mozilla.org/logviewer.html#?job_id=7758967&repo=fx-team

https://hg.mozilla.org/integration/fx-team/rev/4cea7a5931c3

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/6fe81a6dbaa8

Comment 10

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

That test is still failing, so I'm backing out everything from that original push:
https://hg.mozilla.org/integration/fx-team/rev/4d65c8ec069b

Comment 11

[Pulsebot]

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

Comment 12

[Carsten Book [:Tomcat]]

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