Bug 151378

Summary:

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

Product:

WebKit

Reporter:

Nikita Vasilyev <nvasilyev>

Component:

Web Inspector

Assignee:

Devin Rousso <hi>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bburg, buildbot, commit-queue, hi, joepeck, keith_miller, mark.lam, mathias, mattbaker, msaboff, nvasilyev, rniwa, saam, timothy, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Local Build

Hardware:

All

OS:

All

Bug Depends on:

126337

Bug Blocks:

Attachments:

Description	Flags
[Image] Bug	none
Patch	none
Patch	none
Patch	none
Archive of layout-test-results from ews100 for mac-yosemite	none
Archive of layout-test-results from ews105 for mac-yosemite-wk2	none
Archive of layout-test-results from ews115 for mac-yosemite	none
Patch	none
Patch	none
Patch	none
Patch	none
Archive of layout-test-results from ews103 for mac-yosemite	none
Archive of layout-test-results from ews105 for mac-yosemite-wk2	none
Archive of layout-test-results from ews114 for mac-yosemite	none
Patch	none
Patch	joepeck: review+, buildbot: commit-queue-
Archive of layout-test-results from ews102 for mac-yosemite	none
Patch	commit-queue: commit-queue-
Archive of layout-test-results from webkit-cq-03 for mac-yosemite	none
Patch	none

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.