Closed Bug 1838803 Opened 9 months ago Closed 7 months ago

Add indentation and opening/closing brackets for grouped rules

Categories

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

enhancement

Tracking

(firefox117 fixed)

RESOLVED FIXED
117 Branch
Tracking Status
firefox117 --- fixed

People

(Reporter: nchevobbe, Assigned: nchevobbe)

References

(Blocks 2 open bugs, Regressed 1 open bug)

Details

Attachments

(2 files)

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

Blocks: 1838806
Priority: -- → P2
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED

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

Pushed by nchevobbe@mozilla.com:
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.
Regressions: 1845152
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → 117 Branch

== 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?

Flags: needinfo?(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

Flags: needinfo?(nchevobbe)

I profiled the test without and with my patches:

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?

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.

You need to log in before you can comment on or make changes to this bug.