Bug 147362 - Web Inspector: Support smart-pasting in the Rules sidebar panel
Summary: Web Inspector: Support smart-pasting in the Rules sidebar panel
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-07-28 09:57 PDT by Devin Rousso
Modified: 2015-07-30 16:57 PDT (History)
8 users (show)

See Also:


Attachments
Patch (4.29 KB, patch)
2015-07-28 22:39 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (4.95 KB, patch)
2015-07-29 22:17 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (2.70 KB, patch)
2015-07-30 10:07 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (5.63 KB, patch)
2015-07-30 14:35 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2015-07-28 09:57:03 PDT
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.
Comment 1 Radar WebKit Bug Importer 2015-07-28 09:57:14 PDT
<rdar://problem/22031993>
Comment 2 Devin Rousso 2015-07-28 22:39:26 PDT
Created attachment 257735 [details]
Patch
Comment 3 Timothy Hatcher 2015-07-29 08:53:02 PDT
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?
Comment 4 Devin Rousso 2015-07-29 22:17:36 PDT
Created attachment 257815 [details]
Patch
Comment 5 Devin Rousso 2015-07-30 10:07:47 PDT
Created attachment 257838 [details]
Patch
Comment 6 Devin Rousso 2015-07-30 10:08:53 PDT
(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.
Comment 7 Timothy Hatcher 2015-07-30 11:32:10 PDT
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.
Comment 8 Devin Rousso 2015-07-30 14:35:24 PDT
Created attachment 257854 [details]
Patch
Comment 9 WebKit Commit Bot 2015-07-30 16:56:58 PDT
Comment on attachment 257854 [details]
Patch

Clearing flags on attachment: 257854

Committed r187625: <http://trac.webkit.org/changeset/187625>
Comment 10 WebKit Commit Bot 2015-07-30 16:57:02 PDT
All reviewed patches have been landed.  Closing bug.