Closed Bug 1524547 Opened 8 months ago Closed 7 months ago

Export Changes - copy full contents of CSS rule with changes applied

Categories

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

enhancement

Tracking

(firefox67 fixed)

RESOLVED FIXED
Firefox 67
Tracking Status
firefox67 --- fixed

People

(Reporter: rcaliman, Assigned: rcaliman)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Provide a context menu option to copy the complete CSS Rule (all declarations with changes applied) for the target rule in the Changes panel.

This gives users the option to copy the rule and replace it in their code editor.

If the target rule is an ancestor rule (ex: @media, @supports), copy all of its nested child CSS rules touched by changes.

Summary: Export Changes - copy full contents CSS rule with changes applied → Export Changes - copy full contents of CSS rule with changes applied

Depends on D20195

  • adds method to identify a CSS Rule actor by id
  • adds context menu option to Changes panel to allow copying the rule's authored text to the clipboard

This series of patches requires the patch from Bug 1525238 in order to run.

This patch adds a new method to the StyleRuleActor on the server to expose the full text content of a CSS rule over the protocol to the client. This means exposing the CSS rule's authored text including comments and any nested rules if the target rule is an ancestor rule, like @media or @supports.

There's minor refactoring to include additional rule types for which StyleRuleActors will be created: @supports, @media, @keyframes. These are not explicitly rendered in the Rule view and shouldn't impact it. The Rule view filters for CSS Style Rules, element inline style and has special handling for keyframe at-rules.

Assignee: nobody → rcaliman
Status: NEW → ASSIGNED

I feel like we might be a bit light on the unit tests for these export changes method. I would like to see some unit tests added for the export changes functionality we have done either in this bug or in a separate bug.

The same could be said for the new server methods we are adding as well. We previously had an unit test for just the getRuleText method in https://searchfox.org/mozilla-central/source/devtools/server/tests/unit/test_getRuleText.js. I could see us testing the StyleRuleActor in https://searchfox.org/mozilla-central/source/devtools/server/tests/mochitest/test_styles-applied.html. Other examples live in test_styles-X.html in https://searchfox.org/mozilla-central/source/devtools/server/tests/mochitest.

Based on the years of style actor changes, we have been slightly lax on adding unit tests, but I feel this might have been because we have been forgetting about it. I would want us to be a bit more mindful going forward.

Pushed by rcaliman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6c32c874b102
(Part 1) Add method to StyleRuleActor to get complete authored text for a CSS rule. r=gl
https://hg.mozilla.org/integration/autoland/rev/4cd86fe262ec
(Part 2) Add context menu option to copy the contents of a changed CSS rule. r=gl
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67

(In reply to Gabriel [:gl] (ΦωΦ) from comment #3)

I feel like we might be a bit light on the unit tests for these export changes method. I would like to see some unit tests added for the export changes functionality we have done either in this bug or in a separate bug.

I queued the patches to land before reading your comment here. Agreed. We should have tests for these new features. I'll follow-up in a separate bug.

See Also: → 1529532
See Also: → 1529533
You need to log in before you can comment on or make changes to this bug.