709223 - GCLI needs a 'resource' type

Status: RESOLVED FIXED

(DevTools :: General, defect, P2)

Description

[Joe Walker [:jwalker]]

I hacked up a type for the CSS editor which was the loaded CSS sheets. The debugger has an inline type for scripts. What we need is the ability to do:

type: {
  name: 'resource',
  filter: 'script'
}

Or similar.

I'm sure this type of thing is going to pop up in many places.

Comment 1

[Joe Walker [:jwalker]]

While we are at it, perhaps we should have a command to remove new / inject existing resources from / into a page.

Comment 2

[Joe Walker [:jwalker]]

Bug triage, filter on PEGASUS.

Comment 3

[Joe Walker [:jwalker]]

Bug triage, filter on PEGASUS.

Comment 4

[Joe Walker [:jwalker]]

Attachment: upload 1

a.k.a. https://github.com/campd/gcli/pull/19

Comment 5

[Joe Walker [:jwalker]]

https://tbpl.mozilla.org/?tree=Try&rev=e6cb0ce0cbf3

Comment 6

[Joe Walker [:jwalker]]

Attachment: upload 2

Note - the syntax is:
  type: { name: 'resource', include: 'text/css' },

i.e. s/filter/include/

Comment 7

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Joe has mentioned that we should return the stylesheet, script etc. as arg.resource.element ... not sure if he wants us to do this as part of this bug.

Comment 8

[Joe Walker [:jwalker]]

https://tbpl.mozilla.org/?tree=Try&rev=b447debd6053

Comment 9

[Joe Walker [:jwalker]]

Attachment: upload 3

Change the name of the element to 'element'

Comment 10

[Dave Camp (:dcamp)]

Comment on attachment 593117 [details] [diff] [review]
upload 3

Review of attachment 593117 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me, r+ waiting on a response to the xhr-vs-necko comment.

::: browser/devtools/webconsole/gcli.jsm
@@ +764,5 @@
>        cli.unsetEvalFunction();
>        nodetype.unsetDocument();
>        jstype.unsetGlobalObject();
> +      resource.unsetDocument();
> +      resource.clearResourceCache();

Is there ever a time you'd want to unsetDocument() without clearResourceCache()?  Maybe unsetDocument() should clear the resource cache itself?

@@ +3876,5 @@
> +      callback(xhr.responseText);
> +    };
> +    xhr.open('GET', this.element.src, true);
> +    xhr.send();
> +  }

The style editor has code that uses necko directly, and prefers the cache:

http://mxr.mozilla.org/mozilla-central/source/browser/devtools/styleeditor/StyleEditor.jsm#813

Won't be prettier, but has a better chance of matching the script that's actually loaded in the page.

@@ +3924,5 @@
> +
> +/**
> + * A CSS expression that refers to a single node
> + */
> +function ResourceType(typeSpec) {

This comment doesn't seem to match?

Comment 11

[Joe Walker [:jwalker]]

(In reply to Dave Camp (:dcamp) from comment #10)
> Comment on attachment 593117 [details] [diff] [review]
> upload 3
> 
> Review of attachment 593117 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good to me, r+ waiting on a response to the xhr-vs-necko comment.
> 
> ::: browser/devtools/webconsole/gcli.jsm
> @@ +764,5 @@
> >        cli.unsetEvalFunction();
> >        nodetype.unsetDocument();
> >        jstype.unsetGlobalObject();
> > +      resource.unsetDocument();
> > +      resource.clearResourceCache();
> 
> Is there ever a time you'd want to unsetDocument() without
> clearResourceCache()?  Maybe unsetDocument() should clear the resource cache
> itself?

Makes sense. Good spot.

> @@ +3876,5 @@
> > +      callback(xhr.responseText);
> > +    };
> > +    xhr.open('GET', this.element.src, true);
> > +    xhr.send();
> > +  }
> 
> The style editor has code that uses necko directly, and prefers the cache:
> 
> http://mxr.mozilla.org/mozilla-central/source/browser/devtools/styleeditor/
> StyleEditor.jsm#813
> 
> Won't be prettier, but has a better chance of matching the script that's
> actually loaded in the page.

Agreed, and the same goes for scripts too.
I created bug 723889 so we can fix them both together.

> @@ +3924,5 @@
> > +
> > +/**
> > + * A CSS expression that refers to a single node
> > + */
> > +function ResourceType(typeSpec) {
> 
> This comment doesn't seem to match?

Ug. no.

I propose I fix all of these in bug 723889.
Follow along here: https://github.com/joewalker/gcli/commits/resource2-723889

Comment 12

[Joe Walker [:jwalker]]

https://tbpl.mozilla.org/?tree=Try&rev=946d867e53b4

Comment 13

[Joe Walker [:jwalker]]

https://tbpl.mozilla.org/?tree=Fx-Team&rev=3e9fbc7eb447

Comment 14

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/99d735b9cf8c