Bug 17896 – Inspector console should autocomplete the id's in the document when completing document.getElementById

• Component: Web Inspector
• Status: NEW

[Sam Weinig]

It might also be helpful to autocomplete the 'name' attributes when doing document.getElementsByName, the 'class' attributes when doing document.getElementsByClassName, etc.

[Joseph Pecoraro]

Something like this?
http://screencast.com/t/NTI1MmZjZWIt

I played around with this. I'm doing it in a very inefficient way (not caching, rescanning the document every time) and there are magical things with ranges that I work around. But does this sound like something useful?

[Timothy Hatcher]

Hot and Large!

[Pavel Feldman]

(In reply to comment #1)
> Something like this?
> http://screencast.com/t/NTI1MmZjZWIt
> 
> I played around with this. I'm doing it in a very inefficient way (not caching, rescanning the document every time) and there are magical things with ranges that I work around. But does this sound like something useful?

When you are saying "rescanning the document every time", do you mean anything other than 

document.querySelectorAll("[id]");?

[Joseph Pecoraro]

Yes, worse, I was just doing getElementsByTagName("*") for my proof of concept. Your query is much better! But even that might be scanning the full document.

With that suggestion I can probably do the following:

    getElementById         => querySelectorAll("[id]")
    getElementsByClassName => querySelectorAll("[class]")
    getElementsByName      => querySelectorAll("[name]")
    getElementsByTagName   => getElementsByTagName("*") + filter?
    querySelector[All]     => something fancy, like a mix of the above, #id, .class, tagName

I might take a look at:

  - getElementsByTagNameNS => same as above but with namespaces?
  - createElement[NS]      => valid html types, namespaces?
  - createEvent            => valid event types
  - maybe $ and $$         => getElementById && querySelectorAll
  - addEventListener       => event types? functions?
  - removeEventListener    => event types? functions?

Knowing the function you're writing into can be kind of
useful. You can decide only to return objects of a
particular type. So for the 2nd argument of addEventListener
you would want either functions, or object's with a
"handleEvent" property. Thats going a bit overkill
though.

By the way, a did do a quick balanced parenthesis check
so the following will at least let you know that the
function is "getElementById". I haven't played around
with what I want to do in this case.

  document.getElementById( func() + |

[Joseph Pecoraro]

Created attachment 59226 [details]
[PATCH] Work in Progress

Feel free to review, but I won't be satisfied with this until I can automatically provide
a closing ")" in the completion. It appears that the ")" triggers a new completion
because ")" is a special character.

This patch provides simple caching. After a list is fetched, a timer is started to clear
all lists after 30 seconds. I could change to timestamps, or cache indefinitely. Any
preferences?

@Pavel. I make use of "window" and "document" directly in this patch. What
should I be worried about? I recently found out document could be destroyed
if the frame navigates, and access to it will cause a crash. =/

[Joseph Pecoraro]

Created attachment 59228 [details]
[PATCH] Work in Progress (Improved)

>     getElementsByClassName => querySelectorAll("[class]")

Whoops, I forgot to update this one. Fixed.

>   - maybe $ and $$         => getElementById && querySelectorAll

Oversight of mine. Fixed.

[Pavel Feldman]

Comment on attachment 59228 [details]
[PATCH] Work in Progress (Improved)

WebCore/inspector/front-end/ConsoleView.js:353
 +      _getCurrentFunctionName: function(text)
No 'get' prefix in WebKit?

WebCore/inspector/front-end/InjectedScript.js:177
 +      var elems = document.querySelectorAll("[id]");
You are only querying current document, not touching iframes. Imagine the code is frames[0].document.getElementById(...

WebCore/inspector/front-end/InjectedScript.js:190
 +      var elems = document.getElementsByTagName("*");
Same here, this all needs to be done in native code as I've done it to the performSearch.

WebCore/inspector/front-end/InjectedScript.js:235
 +  InjectedScript._generatedCachedQueryObjects = [];
We try to get rid of these...

WebCore/inspector/front-end/InjectedScript.js:380
 +          case "getElementById":
Now this is heuristics. How hard would it be to test whether this function is defined on the document object?

[Joseph Pecoraro]

(In reply to comment #7)
> (From update of attachment 59228 [details])
> WebCore/inspector/front-end/ConsoleView.js:353
>  +      _getCurrentFunctionName: function(text)
> No 'get' prefix in WebKit?

Done.


> WebCore/inspector/front-end/InjectedScript.js:177
>  +      var elems = document.querySelectorAll("[id]");
> You are only querying current document, not touching iframes. Imagine the code is frames[0].document.getElementById(...

Currently we don't even support autocompletion across frames.
So "frames[0].do" wouldn't even complete to "document".
Autocompletion doesn't dig into either "obj[key]" or "obj.get(key)".
I don't know why it doesn't dig into the bracket syntax, but
certainly the call syntax might cause sideaffects if run.

So, that sounds like a different problem that should be tackled.


> WebCore/inspector/front-end/InjectedScript.js:380
>  +          case "getElementById":
> Now this is heuristics. How hard would it be to test whether this function is defined
> on the document object?

I thought about this at the time. I think ideally we should break
this down into 2 parts. With Example Input:

>             document.body.appendChild( a.b.c().d( someVar|

    - Current Typing Unit      =>  someVar

      This can autocomplete to variables and the usual things.
      Combining this with the next part, if we know its a particular
      function we can autocomplete with strings or restrict the
      variables to types that make sense.

    - Previous Unit            =>  a.b.c().d

      This can be checked against a known function, because it is
      being called with "(". Again here, we have a function call c(),
      so to be safe we don't evaluate it. But assuming that part
      wasn't there "a.b.d" we could compare against each frame's
      `document.getElementById` or something like that.

When I wrote this patch I didn't think far enough ahead. I just
wanted to play with it. I've already missed it while working on
my other machines!


> WebCore/inspector/front-end/InjectedScript.js:190
>  +      var elems = document.getElementsByTagName("*");
> Same here, this all needs to be done in native code as I've done it to the performSearch.

I don't know about that. I thought if the user types "document.getElementsByTagName("
they would only care about the tag names of elements in ^^^ that "document". Not inner
<iframe>s which that cannot access. Given:

    <body>
      <iframe src="data:text/html,<p>Hi</p>"></iframe>
      <script>
        setTimeout(function() {
            alert( document.getElementsByTagName("p").length ); // => 0
        }, 1000);
      </script>
    </body>


> WebCore/inspector/front-end/InjectedScript.js:235
>  +  InjectedScript._generatedCachedQueryObjects = [];
> We try to get rid of these...

Okay. I haven't stress tested this to see if caching is even useful.


> NOTE TO SELF: This would also need a test. If performSearch can
> be tested, this most certainly can be!

Noted.

[Timothy Hatcher]

*** Bug 80036 has been marked as a duplicate of this bug. ***

[Umar Hansa]

(In reply to comment #5)
> This patch provides simple caching. After a list is fetched, a timer is started to clear
> all lists after 30 seconds. I could change to timestamps, or cache indefinitely. Any
> preferences?

Hi, I'm not a WebKit committer or anything, but this feature looked interesting so I had a quick look through it, sorry if this comes across naive but instead of clearing the cached queries every 30 seconds would it be worth using DOM Mutation Events (http://www.w3.org/TR/DOM-Level-2-Events/events.html) or Mutation Observers to detect changes to the DOM and then clear the cache?

[Pavel Feldman]

> Hi, I'm not a WebKit committer or anything, but this feature looked interesting so I had a quick look through it, sorry if this comes across naive but instead of clearing the cached queries every 30 seconds would it be worth using DOM Mutation Events (http://www.w3.org/TR/DOM-Level-2-Events/events.html) or Mutation Observers to detect changes to the DOM and then clear the cache?

This patch and the approach is slightly out of date. We no longer like doing non-formatting JavaScript in the context of running page. I would expose "idsForInspector" on the DocumentOrderedMap and would fetch all ids at the time they are needed.

[Radar WebKit Bug Importer]

<rdar://problem/19281596>