RESOLVED FIXED 138812
Web Inspector: Ability to Copy entire CSS Rule from Styles Sidebar 
https://bugs.webkit.org/show_bug.cgi?id=138812
Summary Web Inspector: Ability to Copy entire CSS Rule from Styles Sidebar
Joseph Pecoraro
Reported 2014-11-17 16:25:26 PST
* SUMMARY It would be helpful to copy an entire CSS rule from the sidebar. Currently each Rule's Selector and Properties are separate fields, it would be nice to be able to copy/paste the entire rule (Selector, Properties, and optionally surrounding @media rules block) from the Styles sidebar. * TEST: <style> @media (min-width:600px) { body { color: red; } } </style> <body>Test</body> * STEPS TO REPRODUCE 1. Inspect <body> on test page 2. Show Styles sidebar => Should be able to copy the body styles from the sidebar somehow * NOTES - Would be nice to copy with and without the @media. - Perhaps right clicking anywhere in the selector/property editors?
Attachments
Patch (3.73 KB, patch)
2015-06-04 10:41 PDT, Devin Rousso 		no flags
DetailsFormatted DiffDiff
Patch (3.96 KB, patch)
2015-06-04 14:13 PDT, Devin Rousso 		no flags
DetailsFormatted DiffDiff
Patch (3.94 KB, patch)
2015-06-04 16:19 PDT, Devin Rousso 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2014-11-17 16:25:45 PST
<rdar://problem/19009055>
Timothy Hatcher
Comment 2 2014-11-19 10:44:23 PST
Maybe if the entire selection range of the rule is copied, include the selector too?
Devin Rousso
Comment 3 2015-06-04 10:41:16 PDT
Created attachment 254272 [details] Patch
Timothy Hatcher
Comment 4 2015-06-04 10:46:50 PDT
Comment on attachment 254272 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=254272&action=review I think a context menu on the rule section would be best. > Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationSection.css:70 > + cursor: pointer; Not sure pointer is the right cursor. This might be best as a context menu item. Click to Copy isn't a typical user action in most places. > Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationSection.js:110 > + (function(thisObject) { > + thisObject._iconElement.addEventListener("click", function() { You should use .bind(this) on the event listener function instead. No need for a closure.
Joseph Pecoraro
Comment 5 2015-06-04 12:48:39 PDT
Comment on attachment 254272 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=254272&action=review > Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationSection.js:119 > + styleText += "@media" + thisObject._style.ownerRule.mediaList[0].text + " {\n "; Seems weird that this would always be mediaList[0]. Does that make sense?
Devin Rousso
Comment 6 2015-06-04 14:13:15 PDT
Created attachment 254299 [details] Patch
Joseph Pecoraro
Comment 7 2015-06-04 15:00:08 PDT
Comment on attachment 254299 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=254299&action=review > Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationSection.js:108 > + this._headerElement.addEventListener("contextmenu", function(event) { Style: We would probably have a _handleContextMenuEvent method down below and here we would just have: this._headerElement.addEventListener("contextmenu", this._handleContextMenuEvent.bind(this)); > Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationSection.js:110 > + if (window.getSelection().toString().length) > + return; What is this bail for? > Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationSection.js:115 > + InspectorFrontendHost.copyText(this._generateCSSRuleString.call(this)); Nit: this.foo.call(this) is no different than this.foo(); > Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationSection.js:293 > + _generateCSSRuleString: function() { Style: Brace on new line. > Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationSection.js:301 > + function repeatText(text, numRepeats) { > + var output = ""; > + > + for (var i = 0; i < numRepeats; ++i) > + output += text; > + > + return output; > + } Actually, we support String.prototype.repeat. So you could just do: "test".repeat(3) And get: "testtesttest" > Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationSection.js:312 > + for (var i = numMediaQueries - 1; i >= 0; --i) > + styleText += repeatText(" ", numMediaQueries - i - 1) + "@media " + mediaList[i].text + " {\n"; I wonder if this would be a good place to adopt template strings: styleText += `${" ".repeat(numMediaQueries - i - 1)} @media ${mediaList[i].text} {\n` But maybe I'm just getting too excited. Stick with just strings for now. > Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationSection.js:325 > + if (styleText.slice(-1) !== ";") > + styleText += ";"; I think: !styleText.endsWith(";") is more expressive.
Devin Rousso
Comment 8 2015-06-04 16:11:21 PDT
Comment on attachment 254299 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=254299&action=review >> Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationSection.js:110 >> + return; > > What is this bail for? If part of the selector text is highlighted, don't replace the context menu as someone may want to just copy/paste a selector.
Devin Rousso
Comment 9 2015-06-04 16:19:01 PDT
Created attachment 254311 [details] Patch
WebKit Commit Bot
Comment 10 2015-06-18 13:44:44 PDT
Comment on attachment 254311 [details] Patch Clearing flags on attachment: 254311 Committed r185720: <http://trac.webkit.org/changeset/185720>
WebKit Commit Bot
Comment 11 2015-06-18 13:44:50 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.