1838803 - Add indentation and opening/closing brackets for grouped rules

Status: RESOLVED FIXED

(DevTools :: Inspector: Rules, enhancement, P2)

Description

[Nicolas Chevobbe [:nchevobbe]]

At the moment, the following CSS:

body {
  #div {
    color: red;
    @media screen {
      background: black;
    }
  }
}

will be displayed in the rule view like that:

body
#div
@media screen
& {
      background: black;
}

but it would make more sense to stay closer to how the CSS might be authored, and have:

body {
  #div {
    @media screen {
      & {
        background: black;
      }
    }
  }
}

this might take a bit of vertical space, so we could offer a mechanism to toggle between a "compact" and a "verbose" mode

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1838803 - [devtools] Add opening/closing brackets & indentation for nested rules. r=#devtools-reviewers.

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1838803 - [devtools] Handle copy/pasting of nested rule. r=#devtools-reviewers.

Make sure that selecting a rule does copy the rule with the expected indentation.
Some ul and ol lists are turned into div with a list role (same of li
which are turned into div with listitem role) so the engine won't add the
regular indent it usually adds.

Depends on D182441

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d028a76dffe5
[devtools] Add opening/closing brackets & indentation for nested rules. r=devtools-reviewers,bomsy.
https://hg.mozilla.org/integration/autoland/rev/9e4ff40d2afa
[devtools] Handle copy/pasting of nested rule. r=devtools-reviewers,bomsy.

Comment 4

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/d028a76dffe5
https://hg.mozilla.org/mozilla-central/rev/9e4ff40d2afa

Comment 5

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/d028a76dffe5
https://hg.mozilla.org/mozilla-central/rev/9e4ff40d2afa

Comment 6

[Julian Descottes [:jdescottes]]

== Change summary for alert #39152 (as of Wed, 26 Jul 2023 10:27:36 GMT) ==

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)
25%	damp custom.inspector.manyrules.deselectnode	windows10-64-shippable-qr	e10s fission stylo webrender-sw	166.35 -> 208.25
25%	damp custom.inspector.manyrules.deselectnode	windows10-64-shippable-qr	e10s fission stylo webrender	166.43 -> 207.91
24%	damp custom.inspector.manyrules.deselectnode	linux1804-64-shippable-qr	e10s fission stylo webrender-sw	201.80 -> 251.18
24%	damp custom.inspector.manyrules.deselectnode	linux1804-64-shippable-qr	e10s fission stylo webrender	204.24 -> 253.33

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=39152

Nicolas, do you think the performance regression above could be linked to this change?

Comment 7

[Nicolas Chevobbe [:nchevobbe]]

The TRY push doesn't seem to have anything else that would lead to such numbers, so yes, it's very likely.
Not sure why this impacts deselectnode though, I'll have a look

Comment 8

[Nicolas Chevobbe [:nchevobbe]]

I profiled the test without and with my patches:

• before: https://share.firefox.dev/3KixZ3y
• after: https://share.firefox.dev/3KIux2L

I don't see any obvious function calls that would explain this difference. The only thing might be more time spent in reflow and style, which may be caused because of more dom elements or/and non-efficient styling?

Comment 9

[Nicolas Chevobbe [:nchevobbe]]

https://hg.mozilla.org/mozilla-central/rev/9e4ff40d2afa seems to be the culprit here.
It does create spans before each property where we were only relying on the indent provided by the li before.