151378 – Web Inspector: Some CSS selectors in the UI aren't escaped

RESOLVED FIXED 151378

Web Inspector: Some CSS selectors in the UI aren't escaped

https://bugs.webkit.org/show_bug.cgi?id=151378

Summary Web Inspector: Some CSS selectors in the UI aren't escaped

Nikita Vasilyev

Reported 2015-11-17 19:37:07 PST

Created attachment 265729 [details] [Image] Bug This is a valid HTML with a valid id name: <div id="#id.class"> To match this id using CSS, I'd have to escape it: #\#id\.class We escape CSS selectors in some parts of Web Inspector's UI and we don't in some others. More on this: https://mothereff.in/css-escapes#0%23id.class

Attachments
[Image] Bug (97.14 KB, image/png) 2015-11-17 19:37 PST, Nikita Vasilyev	no flags	Details
Patch (16.95 KB, patch) 2016-08-13 23:37 PDT, Devin Rousso	no flags	Details Formatted Diff Diff
Patch (28.30 KB, patch) 2016-08-16 01:43 PDT, Devin Rousso	no flags	Details Formatted Diff Diff
Patch (28.23 KB, patch) 2016-08-16 22:42 PDT, Devin Rousso	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews100 for mac-yosemite (740.24 KB, application/zip) 2016-08-16 23:29 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews105 for mac-yosemite-wk2 (937.70 KB, application/zip) 2016-08-16 23:32 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews115 for mac-yosemite (1.45 MB, application/zip) 2016-08-16 23:52 PDT, Build Bot	no flags	Details
Patch (28.95 KB, patch) 2016-08-17 17:35 PDT, Devin Rousso	no flags	Details Formatted Diff Diff
Patch (39.52 KB, patch) 2016-08-18 18:49 PDT, Devin Rousso	no flags	Details Formatted Diff Diff
Patch (40.16 KB, patch) 2016-08-23 10:30 PDT, Devin Rousso	no flags	Details Formatted Diff Diff
Patch (24.15 KB, patch) 2016-08-23 14:31 PDT, Devin Rousso	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews103 for mac-yosemite (1.22 MB, application/zip) 2016-08-23 15:17 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews105 for mac-yosemite-wk2 (811.98 KB, application/zip) 2016-08-23 17:45 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews114 for mac-yosemite (1.67 MB, application/zip) 2016-08-23 18:33 PDT, Build Bot	no flags	Details
Patch (24.02 KB, patch) 2016-08-24 22:52 PDT, Devin Rousso	no flags	Details Formatted Diff Diff
Patch (28.35 KB, patch) 2016-08-25 18:29 PDT, Devin Rousso	joepeck: review+ buildbot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ews102 for mac-yosemite (895.46 KB, application/zip) 2016-08-25 19:20 PDT, Build Bot	no flags	Details
Patch (27.11 KB, patch) 2016-08-25 21:45 PDT, Devin Rousso	commit-queue: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from webkit-cq-03 for mac-yosemite (895.32 KB, application/zip) 2016-08-26 01:15 PDT, WebKit Commit Bot	no flags	Details
Patch (27.17 KB, patch) 2016-08-26 10:35 PDT, Devin Rousso	no flags	Details Formatted Diff Diff
Show Obsolete (18) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2015-11-17 19:37:56 PST

<rdar://problem/23588196>

Devin Rousso

Comment 2 2016-08-13 23:37:34 PDT

Created attachment 286020 [details] Patch

Joseph Pecoraro

Comment 3 2016-08-15 12:14:26 PDT

Comment on attachment 286020 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=286020&action=review Patch looks good, but a comment below on how it might be too general. > Source/WebCore/inspector/InspectorOverlayPage.js:268 > + function escapeCharacters(string) { > + return string.replace(/([^\w\-\s])/g, "\\$1"); > + } While nice and general, I don't think this gives the best experience for many non-ascii characters. For example, if my className is: áwésómé Then escapeCharacters(className) produces: "\\áw\\és\\óm\\é" Which is not necessary here, since these unicode characters would have worked unescaped. Perhaps we want to restrict this to escaping the smaller set of ambiguous control characters. For example: #.:[]="'>| There may be others, this is just a small sample. > Source/WebInspectorUI/UserInterface/Base/DOMUtilities.js:41 > + var id = node.getAttribute("id"); > + if (id && id.trim().length) { Perhaps we should always do the trim? var id = node.getAttribute("id"); if (id) { id = id.trim(); if (id.length) { ... } } > Source/WebInspectorUI/UserInterface/Base/DOMUtilities.js:43 > + title += "[id=\"" + id.replace(/(\\|\")/g, "\\$1") + "\"]"; If escapeCharacters is made to only handle the control characters, it could be used here. > Source/WebInspectorUI/UserInterface/Base/DOMUtilities.js:49 > + if (className && className.trim().length) { Ditto for the trim.

Nikita Vasilyev

Comment 4 2016-08-15 12:23:37 PDT

Should we just use CSS.escape polyfill? https://github.com/mathiasbynens/CSS.escape Also, you should include a comment "// FIXME: Use CSS.escape once it's implemented http://webkit.org/b/126337" or something along these lines.

Devin Rousso

Comment 5 2016-08-15 13:16:58 PDT

(In reply to comment #4) > Should we just use CSS.escape polyfill? > https://github.com/mathiasbynens/CSS.escape I like this idea. Is there a particular way in which we want to deal with polyfills inside WebInspector? I was thinking something along the lines of creating a UserInterface/Base/Polyfills.js and having all polyfills be located there.

Joseph Pecoraro

Comment 6 2016-08-15 19:13:02 PDT

(In reply to comment #5) > (In reply to comment #4) > > Should we just use CSS.escape polyfill? > > https://github.com/mathiasbynens/CSS.escape > > I like this idea. Is there a particular way in which we want to deal with > polyfills inside WebInspector? I was thinking something along the lines of > creating a UserInterface/Base/Polyfills.js and having all polyfills be > located there. Sounds good to me!

Nikita Vasilyev

Comment 7 2016-08-16 00:19:21 PDT

(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > Should we just use CSS.escape polyfill? > > > https://github.com/mathiasbynens/CSS.escape > > > > I like this idea. Is there a particular way in which we want to deal with > > polyfills inside WebInspector? I was thinking something along the lines of > > creating a UserInterface/Base/Polyfills.js and having all polyfills be > > located there. > > Sounds good to me! I was thinking of putting it in UserInterface/External. One file per polyfill since there could be different licenses and coding styles.

Devin Rousso

Comment 8 2016-08-16 01:43:33 PDT

Created attachment 286169 [details] Patch

Devin Rousso

Comment 9 2016-08-16 22:42:12 PDT

Created attachment 286276 [details] Patch

WebKit Commit Bot

Comment 10 2016-08-16 22:43:56 PDT

Attachment 286276 [details] did not pass style-queue: ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:3: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:4: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:5: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:6: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:7: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:8: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:9: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:10: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:11: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:12: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:13: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:16: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:17: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:18: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:20: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:21: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:22: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:23: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:24: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:25: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:26: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:27: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:28: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:29: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:30: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:31: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:33: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:34: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:35: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:36: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:37: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:38: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:40: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:41: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:42: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:43: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:44: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:45: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:46: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:47: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:48: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:49: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:50: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:51: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:52: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:53: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:54: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:55: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:56: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:57: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:58: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:60: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:61: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:62: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:63: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:64: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:65: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:66: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:67: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:68: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:69: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:71: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:72: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:73: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:74: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:75: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:76: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:77: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:78: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:79: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:80: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:81: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:82: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:83: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:84: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:85: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:86: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:88: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:89: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:90: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:92: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:93: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:94: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:96: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:97: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:98: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:100: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:101: Line contains tab character. [whitespace/tab] [5] Total errors found: 88 in 20 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 11 2016-08-16 23:29:53 PDT

Comment on attachment 286276 [details] Patch Attachment 286276 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1885509 New failing tests: inspector/css/generate-css-rule-string.html

Build Bot

Comment 12 2016-08-16 23:29:57 PDT

Created attachment 286280 [details] Archive of layout-test-results from ews100 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-yosemite Platform: Mac OS X 10.10.5

Build Bot

Comment 13 2016-08-16 23:32:33 PDT

Comment on attachment 286276 [details] Patch Attachment 286276 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/1885511 New failing tests: inspector/css/generate-css-rule-string.html

Build Bot

Comment 14 2016-08-16 23:32:37 PDT

Created attachment 286281 [details] Archive of layout-test-results from ews105 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5

Build Bot

Comment 15 2016-08-16 23:52:09 PDT

Comment on attachment 286276 [details] Patch Attachment 286276 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1885522 New failing tests: inspector/css/generate-css-rule-string.html

Build Bot

Comment 16 2016-08-16 23:52:14 PDT

Created attachment 286283 [details] Archive of layout-test-results from ews115 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-yosemite Platform: Mac OS X 10.10.5

Devin Rousso

Comment 17 2016-08-17 17:35:37 PDT

Created attachment 286348 [details] Patch

WebKit Commit Bot

Comment 18 2016-08-17 17:37:40 PDT

Attachment 286348 [details] did not pass style-queue: ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:3: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:4: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:5: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:6: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:7: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:8: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:9: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:10: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:11: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:12: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:13: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:16: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:17: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:18: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:20: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:21: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:22: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:23: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:24: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:25: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:26: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:27: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:28: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:29: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:30: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:31: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:33: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:34: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:35: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:36: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:37: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:38: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:40: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:41: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:42: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:43: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:44: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:45: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:46: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:47: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:48: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:49: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:50: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:51: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:52: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:53: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:54: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:55: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:56: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:57: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:58: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:60: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:61: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:62: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:63: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:64: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:65: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:66: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:67: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:68: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:69: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:71: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:72: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:73: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:74: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:75: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:76: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:77: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:78: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:79: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:80: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:81: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:82: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:83: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:84: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:85: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:86: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:88: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:89: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:90: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:92: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:93: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:94: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:96: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:97: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:98: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:100: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/css.escape.js:101: Line contains tab character. [whitespace/tab] [5] Total errors found: 88 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.

Joseph Pecoraro

Comment 19 2016-08-18 13:43:56 PDT

Comment on attachment 286348 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=286348&action=review r- for the complex build issues. (I didn't look at the other details of the patch too closely yet) > Source/WebCore/inspector/InspectorOverlayPage.html:33 > <script src="InspectorOverlayPage.js"></script> > +<script src="../../WebInspectorUI/UserInterface/External/css.escape.js"></script> Nit: This include should be before InspectorOverlayPage.js I don't think this is will work with Apple's build system. My understanding is that Apple's build system builds each Source/<X> directory in isolation. So that means when building Source/WebCore it is not guaranteed that "../../WebInspectorUI" exists. This is a build time concern because this file is processed at build time, with the scripts and stylesheets inlined. See DerivedSources.make: all : InspectorOverlayPage.h InspectorOverlayPage.h : InspectorOverlayPage.html InspectorOverlayPage.css InspectorOverlayPage.js $(PYTHON) $(JavaScriptCore_SCRIPTS_DIR)/inline-and-minify-stylesheets-and-scripts.py $(WebCore)/inspector/InspectorOverlayPage.html ./InspectorOverlayPage.combined.html $(PERL) $(JavaScriptCore_SCRIPTS_DIR)/xxd.pl InspectorOverlayPage_html ./InspectorOverlayPage.combined.html InspectorOverlayPage.h $(DELETE) InspectorOverlayPage.combined.html Even if it wasn't at build time, the relative path wouldn't make sense on macOS where WebCore and WebInspectorUI are ultimate packaged into frameworks. It is a complex process when we need to share a resource across project boundaries (such as WebCore and WebInspectorUI here). In most cases we manage this by "exporting" something from a framework via its PrivateHeaders. We do that for example with WebInspector Scripts (jsmin / cssmin). Here however, I don't think the complexity is warranted. I suggest just adding "css.escape.js" to both WebCore and WebInspectorUI. It is small, and can eventually be removed when WebKit natively implements it! - Source/WebCore/inspector/css.escape.js - Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js > Source/WebInspectorUI/UserInterface/External/css.escape.js:2 > +/*! https://mths.be/cssescape v1.5.0 by @mathias | MIT license */ > +;(function(root, factory) { Take a look at Source/WebInspectorUI/Scripts/copy-user-interface-resources.pl to see how we minify and package External scripts for Production builds. It would be nice to have this like: External/CSSEscape - css.escape.js - LICENSE And get the same minification support as the other external libraries. The repository already has a LICENSE-MIT.txt that you can use <https://github.com/mathiasbynens/CSS.escape/blob/master/LICENSE-MIT.txt>. > Source/WebInspectorUI/UserInterface/Test.html:38 > <script src="External/Esprima/esprima.js"></script> > > + <script src="External/css.escape.js"></script> Lets group these more tightly.

Devin Rousso

Comment 20 2016-08-18 18:49:24 PDT

Created attachment 286427 [details] Patch

WebKit Commit Bot

Comment 21 2016-08-18 18:50:49 PDT

Attachment 286427 [details] did not pass style-queue: ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:3: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:4: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:5: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:6: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:7: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:8: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:9: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:10: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:11: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:12: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:13: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:16: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:17: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:18: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:20: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:21: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:22: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:23: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:24: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:25: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:26: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:27: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:28: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:29: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:30: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:31: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:33: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:34: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:35: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:36: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:37: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:38: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:40: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:41: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:42: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:43: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:44: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:45: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:46: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:47: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:48: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:49: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:50: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:51: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:52: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:53: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:54: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:55: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:56: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:57: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:58: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:60: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:61: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:62: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:63: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:64: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:65: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:66: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:67: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:68: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:69: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:71: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:72: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:73: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:74: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:75: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:76: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:77: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:78: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:79: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:80: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:81: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:82: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:83: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:84: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:85: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:86: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:88: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:89: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:90: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:92: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:93: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:94: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:96: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:97: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:98: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:100: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebCore/inspector/External/CSSEscape/css.escape.js:101: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:3: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:4: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:5: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:6: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:7: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:8: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:9: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:10: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:11: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:12: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:13: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:16: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:17: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:18: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:20: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:21: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:22: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:23: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:24: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:25: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:26: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:27: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:28: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:29: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:30: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:31: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:33: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:34: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:35: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:36: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:37: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:38: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:40: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:41: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:42: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:43: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:44: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:45: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:46: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:47: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:48: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:49: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:50: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape .js:51: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:52: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:53: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:54: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:55: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:56: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:57: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:58: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:60: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:61: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:62: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:63: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:64: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:65: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:66: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:67: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:68: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:69: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:71: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:72: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:73: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:74: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:75: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:76: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:77: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:78: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:79: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:80: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:81: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:82: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:83: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:84: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:85: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:86: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:88: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:89: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:90: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:92: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:93: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:94: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:96: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:97: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:98: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:100: Line contains tab character. [whitespace/tab] [5] ERROR: Source/WebInspectorUI/UserInterface/External/CSSEscape/css.escape.js:101: Line contains tab character. [whitespace/tab] [5] Total errors found: 176 in 25 files If any of these errors are false positives, please file a bug against check-webkit-style.

Joseph Pecoraro

Comment 22 2016-08-22 21:11:18 PDT

Comment on attachment 286427 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=286427&action=review Patch looks good to me. We should get a nod from someone on importing this MIT licensed code. Put up 1 more patch (removing the tabs) and I'll get someone to okay it. > Source/WebCore/inspector/External/CSSEscape/css.escape.js:1 > +/*! https://mths.be/cssescape v1.5.0 by @mathias | MIT license */ I think you do want to replace tabs with spaces. Otherwise I think you won't be able to commit. Also in WebCore this can probably just be css.escape.js without the extra directories. Remember, it should eventually be dropped in favor of a native impl. > Source/WebInspectorUI/UserInterface/Models/DOMNode.js:550 > + let foundClasses = new Set; Does this need to normalize upper/lowercase? It didn't before, so it is fine now, but it might be worth changing.

Joseph Pecoraro

Comment 23 2016-08-22 21:11:38 PDT

Comment on attachment 286427 [details] Patch r- while waiting for updated patch.

Devin Rousso

Comment 24 2016-08-23 10:28:24 PDT

(In reply to comment #22) > > Source/WebInspectorUI/UserInterface/Models/DOMNode.js:550 > > + let foundClasses = new Set; > > Does this need to normalize upper/lowercase? It didn't before, so it is fine > now, but it might be worth changing. I don't really think this is easily doable. Since something like <div class="test Test tEsT"> would be matched by `.TeSt` no matter which of the three classes are applied to the <div>, which of "test", "Test", and "tEsT" would we want to display as the normalized value?

Devin Rousso

Comment 25 2016-08-23 10:30:06 PDT

Created attachment 286727 [details] Patch

Joseph Pecoraro

Comment 26 2016-08-23 14:02:35 PDT

(In reply to comment #25) > Created attachment 286727 [details] > Patch After looking into it, implementing CSS.escape should be rather trivial for us. So I put a patch up on bug 126337. Lets wait for that and then we can avoid all the polyfill complexity. Sorry for all the hassle =/

Devin Rousso

Comment 27 2016-08-23 14:31:51 PDT

Created attachment 286777 [details] Patch

Build Bot

Comment 28 2016-08-23 15:16:55 PDT

Comment on attachment 286777 [details] Patch Attachment 286777 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1930214 New failing tests: inspector/css/generate-css-rule-string.html

Build Bot

Comment 29 2016-08-23 15:17:00 PDT

Created attachment 286787 [details] Archive of layout-test-results from ews103 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-yosemite Platform: Mac OS X 10.10.5

Build Bot

Comment 30 2016-08-23 17:45:28 PDT

Comment on attachment 286777 [details] Patch Attachment 286777 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/1930973 New failing tests: inspector/css/generate-css-rule-string.html

Build Bot

Comment 31 2016-08-23 17:45:32 PDT

Created attachment 286810 [details] Archive of layout-test-results from ews105 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5

Build Bot

Comment 32 2016-08-23 18:33:09 PDT

Comment on attachment 286777 [details] Patch Attachment 286777 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1931127 New failing tests: inspector/css/generate-css-rule-string.html

Build Bot

Comment 33 2016-08-23 18:33:13 PDT

Created attachment 286814 [details] Archive of layout-test-results from ews114 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-yosemite Platform: Mac OS X 10.10.5

Devin Rousso

Comment 34 2016-08-24 22:52:14 PDT

Created attachment 286946 [details] Patch

Joseph Pecoraro

Comment 35 2016-08-25 12:10:40 PDT

Comment on attachment 286946 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=286946&action=review r- only because LayoutTests/inspector/dom/content-node-region-info.html doesn't seem to have been updated. > Source/WebCore/inspector/InspectorOverlayPage.js:273 > + builder.appendSpan("class-name", _truncateString(classes.map(className => "." + CSS.escape(className)).join(""), 50)); Style: Our style for arrow functions is to always wrap parameters in ()s. className => "." + CSS.escape(className) (className) => "." + CSS.escape(className) > Source/WebInspectorUI/UserInterface/Base/DOMUtilities.js:-32 > -WebInspector.displayNameForNode = function(node) This function is used by a test. This test will need to be updated: LayoutTests/inspector/dom/content-node-region-info.html 54: InspectorTest.log("Regions: " + (result.regions ? result.regions.map(WebInspector.displayNameForNode).join(", ") : "N/A")); > Source/WebInspectorUI/UserInterface/Models/DOMNode.js:523 > + get escapedIdAttribute() Maybe this name should be: escapedIdSelector. The "id attribute" doesn't always contain "#" in front. > Source/WebInspectorUI/UserInterface/Models/DOMNode.js:540 > + get escapedClassAttribute() Maybe this name should be: escapedClassSelector. The "class attribute" doesn't always contain "."s. > LayoutTests/inspector/dom/highlightNode-expected.txt:11 > -- Running test case: MainFrameNodeViaNodeId > PASS: Should be one highlighted node. > -Highlighted Element Data: {"tagName":"div","idValue":"id-one","className":".class-two","size":{"width":100,"height":200},"role":""} > +Highlighted Element Data: {"tagName":"div","idValue":"id-one","classes":["class-two"],"size":{"width":100,"height":200},"role":""} It would be good to test an element with multiple class names. And ideally a ::before/::after element.

Devin Rousso

Comment 36 2016-08-25 18:21:39 PDT

(In reply to comment #35) > It would be good to test an element with multiple class names. And ideally a > ::before/::after element. I would agree, but it doesn't look like we can do this with the current logic used by InspectorDOMAgent::highlightSelector, since it relies upon querySelectorAll, which will not return ::before/::after elements.

Devin Rousso

Comment 37 2016-08-25 18:29:57 PDT

Created attachment 287056 [details] Patch

Build Bot

Comment 38 2016-08-25 19:20:02 PDT

Comment on attachment 287056 [details] Patch Attachment 287056 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1943090 New failing tests: imported/w3c/web-platform-tests/html/dom/interfaces.html

Build Bot

Comment 39 2016-08-25 19:20:07 PDT

Created attachment 287065 [details] Archive of layout-test-results from ews102 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-yosemite Platform: Mac OS X 10.10.5

Joseph Pecoraro

Comment 40 2016-08-25 19:21:46 PDT

Comment on attachment 287056 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=287056&action=review r=me > LayoutTests/inspector/dom/highlightSelector.html:142 > + <div class="class-one class-three" style="width: 10px; height: 20px"></div> You should have included a class name that needed to be escaped!

Devin Rousso

Comment 41 2016-08-25 21:45:52 PDT

Created attachment 287074 [details] Patch

WebKit Commit Bot

Comment 42 2016-08-26 01:00:07 PDT

The commit-queue encountered the following flaky tests while processing attachment 287074 [details]: The commit-queue is continuing to process your patch.

WebKit Commit Bot

Comment 43 2016-08-26 01:00:12 PDT

The commit-queue encountered the following flaky tests while processing attachment 287074 [details]: The commit-queue is continuing to process your patch.

WebKit Commit Bot

Comment 44 2016-08-26 01:15:49 PDT

Comment on attachment 287074 [details] Patch Rejecting attachment 287074 [details] from commit-queue. New failing tests: inspector/dom/highlightSelector.html imported/w3c/web-platform-tests/html/dom/interfaces.html Full output: http://webkit-queues.webkit.org/results/1945242

WebKit Commit Bot

Comment 45 2016-08-26 01:15:54 PDT

Created attachment 287078 [details] Archive of layout-test-results from webkit-cq-03 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the commit-queue. Bot: webkit-cq-03 Port: mac-yosemite Platform: Mac OS X 10.10.5

Devin Rousso

Comment 46 2016-08-26 10:35:24 PDT

Created attachment 287119 [details] Patch Fixed escaped selector issue.

WebKit Commit Bot

Comment 47 2016-08-26 13:31:04 PDT

Comment on attachment 287119 [details] Patch Clearing flags on attachment: 287119 Committed r205035: <http://trac.webkit.org/changeset/205035>

WebKit Commit Bot

Comment 48 2016-08-26 13:31:13 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.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version WebKit Local Build

Hardware All

OS All

Product WebKit

Component Web Inspector

Assignee

Devin Rousso

Reported

2015-11-17 19:37 PST

Modified

2016-08-26 13:31 PDT History

CC List

15 users Show

URL

Keywords InRadar

Depends on

126337

Blocks

Dependencies

tree graph