Bug 147362 – Web Inspector: Support smart-pasting in the Rules sidebar panel

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Devin Rousso]

If the user copies an entire rule from a CSS file, the copied text should be formatted when pasting so that the selector is placed in the header and the properties are in the content.  This should make CSS editing from files much easier.

[Radar WebKit Bug Importer]

<rdar://problem/22031993>

[Devin Rousso]

Created attachment 257735 [details]
Patch

[Timothy Hatcher]

Comment on attachment 257735 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=257735&action=review

> Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js:270
> +            this.refresh();

markUndoable here too?

> Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js:278
> +            if (!CSSAgent.setStyleText) {

You should add a compatibility comment, so we can remove this later when we don't support that iOS version anymore.

> Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js:284
> +            CSSAgent.setStyleText(rulePayload.style.styleId, text, styleChanged.bind(this));

If text is empty, should we early return before here?

> Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationSection.js:417
> +        this._style.nodeStyles.addRuleWithSelectorAndText(match[1].trimRight(), match[2].trim());

It is kind of odd this makes a new rule instead of replacing the one you are pasting into. Why not just replace?

[Devin Rousso]

Created attachment 257815 [details]
Patch

[Devin Rousso]

Created attachment 257838 [details]
Patch

[Devin Rousso]

(In reply to comment #5)
> Created attachment 257838 [details]
> Patch

It makes more sense to me that the pasted data does not immediately go into effect, but rather waits on the user to blur the selector editor or press enter.  The style text itself is changed and committed, but the selector is not.

[Timothy Hatcher]

Comment on attachment 257838 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=257838&action=review

> Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationSection.js:419
> +        this._selectorElement.textContent = match[1].trimRight();

I think committing on paste is better, since you commit the style body. Not committing both can lead to a style being applied to some elements when it is meant for another set of elements. That could lead to developer confusion when they don't blur the field.

> Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationSection.js:420
> +        this._style.text = match[2].trim();

You don't need to trim. The whitespace will be stripped (and remembered once we fix bug 145679).

This will commit the change, while the selector change will not.

[Devin Rousso]

Created attachment 257854 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 257854 [details]
Patch

Clearing flags on attachment: 257854

Committed r187625: <http://trac.webkit.org/changeset/187625>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.