Bug 146964 – Web Inspector: update $$() to return an Array

• Component: Web Inspector
• Status: RESOLVED FIXED

[Masataka Yakura]

Lately Mozilla changed their `$$()` function to return an Array instead of NoteList [1] and Chromium followed [2].
It would be great if Web Inspector could do the same!

[1] http://hg.mozilla.org/mozilla-central/rev/c71df86eb2f7
[2] https://chromium.googlesource.com/chromium/blink/+/23126107f2

[Radar WebKit Bug Importer]

<rdar://problem/21831568>

[Joseph Pecoraro]

Sounds good. It also helps us catch an issue with $$'s second parameter causing an exception.

[Joseph Pecoraro]

Created attachment 256850 [details]
[PATCH] Proposed Fix

[Brian Burg]

Comment on attachment 256850 [details]
[PATCH] Proposed Fix

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

r=me

> LayoutTests/inspector/console/command-line-api-expected.txt:31
> +[object HTMLParagraphElement],[object HTMLParagraphElement]

Are NodeList instances printed out any differently than Array instances?

> LayoutTests/inspector/console/command-line-api.html:23
> +    {

nit, should be on same line?

> LayoutTests/inspector/console/command-line-api.html:50
> +                InspectorTest.log("DOM node not found.");

It seems like this case should be a test failure, right? I guess we don't have InspectorTest.error(), though throwing an exception should have the same effect as this.

[Joseph Pecoraro]

Comment on attachment 256850 [details]
[PATCH] Proposed Fix

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

> LayoutTests/inspector/console/command-line-api-expected.txt:16
> +STEP: String($0)
> +[object HTMLParagraphElement]
> +CONSOLE: The console function $() has changed from $=getElementById(id) to $=querySelector(selector). You might try $("#%s")
> +
> +STEP: String($('foo'))
> +null

I'm going to try and update the test to reorder this. I want the CONSOLE messages to be next to the $('foo') line.

>> LayoutTests/inspector/console/command-line-api-expected.txt:31
>> +[object HTMLParagraphElement],[object HTMLParagraphElement]
> 
> Are NodeList instances printed out any differently than Array instances?

Yep, a NodeList would have been printed out like [object NodeList]

>> LayoutTests/inspector/console/command-line-api.html:50
>> +                InspectorTest.log("DOM node not found.");
> 
> It seems like this case should be a test failure, right? I guess we don't have InspectorTest.error(), though throwing an exception should have the same effect as this.

Yeah, it won't match the results. This could be InspectorTest.assert(false, "DOM Node not found") but in any case this is just to detect an error that shouldn't happen.

[Joseph Pecoraro]

Created attachment 256888 [details]
[PATCH] For Landing

[WebKit Commit Bot]

Comment on attachment 256888 [details]
[PATCH] For Landing

Rejecting attachment 256888 [details] from commit-queue.

New failing tests:
inspector/console/command-line-api.html
Full output: http://webkit-queues.appspot.com/results/6394659504914432

[WebKit Commit Bot]

Created attachment 256890 [details]
Archive of layout-test-results from webkit-cq-02 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: webkit-cq-02  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Joseph Pecoraro]

(In reply to comment #8)
> Created attachment 256890 [details]
> Archive of layout-test-results from webkit-cq-02 for mac-mavericks
> 
> The attached test failures were seen while running run-webkit-tests on the
> commit-queue.
> Bot: webkit-cq-02  Port: mac-mavericks  Platform: Mac OS X 10.9.5

lol, I forgot to update the results.

[Joseph Pecoraro]

Created attachment 256892 [details]
[PATCH] For Landing

[WebKit Commit Bot]

Comment on attachment 256892 [details]
[PATCH] For Landing

Clearing flags on attachment: 256892

Committed r186891: <http://trac.webkit.org/changeset/186891>