Last Comment Bug 709223 - GCLI needs a 'resource' type
: GCLI needs a 'resource' type
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: unspecified
: All All
: P2 normal (vote)
: Firefox 13
Assigned To: Joe Walker [:jwalker] (needinfo me or ping on irc)
:
: J. Ryan Stinnett [:jryans] (use ni?)
Mentors:
Depends on:
Blocks: 683499
  Show dependency treegraph
 
Reported: 2011-12-09 11:21 PST by Joe Walker [:jwalker] (needinfo me or ping on irc)
Modified: 2012-02-07 11:25 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
upload 1 (36.56 KB, patch)
2012-01-26 04:11 PST, Joe Walker [:jwalker] (needinfo me or ping on irc)
no flags Details | Diff | Splinter Review
upload 2 (37.23 KB, patch)
2012-01-27 05:34 PST, Joe Walker [:jwalker] (needinfo me or ping on irc)
no flags Details | Diff | Splinter Review
upload 3 (30.08 KB, patch)
2012-01-31 08:52 PST, Joe Walker [:jwalker] (needinfo me or ping on irc)
dcamp: feedback+
Details | Diff | Splinter Review

Description Joe Walker [:jwalker] (needinfo me or ping on irc) 2011-12-09 11:21:49 PST
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] (needinfo me or ping on irc) 2011-12-13 02:25:10 PST
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] (needinfo me or ping on irc) 2012-01-11 10:16:07 PST
Bug triage, filter on PEGASUS.
Comment 3 Joe Walker [:jwalker] (needinfo me or ping on irc) 2012-01-11 10:18:11 PST
Bug triage, filter on PEGASUS.
Comment 4 Joe Walker [:jwalker] (needinfo me or ping on irc) 2012-01-26 04:11:27 PST
Created attachment 591752 [details] [diff] [review]
upload 1

a.k.a. https://github.com/campd/gcli/pull/19
Comment 5 Joe Walker [:jwalker] (needinfo me or ping on irc) 2012-01-27 04:37:36 PST
https://tbpl.mozilla.org/?tree=Try&rev=e6cb0ce0cbf3
Comment 6 Joe Walker [:jwalker] (needinfo me or ping on irc) 2012-01-27 05:34:12 PST
Created attachment 592106 [details] [diff] [review]
upload 2

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

i.e. s/filter/include/
Comment 7 Michael Ratcliffe [:miker] [:mratcliffe] 2012-01-30 03:52:47 PST
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] (needinfo me or ping on irc) 2012-01-31 08:33:04 PST
https://tbpl.mozilla.org/?tree=Try&rev=b447debd6053
Comment 9 Joe Walker [:jwalker] (needinfo me or ping on irc) 2012-01-31 08:52:23 PST
Created attachment 593117 [details] [diff] [review]
upload 3

Change the name of the element to 'element'
Comment 10 Dave Camp (:dcamp) 2012-02-02 17:25:59 PST
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] (needinfo me or ping on irc) 2012-02-03 04:28:28 PST
(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] (needinfo me or ping on irc) 2012-02-06 07:07:00 PST
https://tbpl.mozilla.org/?tree=Try&rev=946d867e53b4
Comment 13 Joe Walker [:jwalker] (needinfo me or ping on irc) 2012-02-07 03:37:37 PST
https://tbpl.mozilla.org/?tree=Fx-Team&rev=3e9fbc7eb447
Comment 14 Tim Taubert [:ttaubert] 2012-02-07 11:25:55 PST
https://hg.mozilla.org/mozilla-central/rev/99d735b9cf8c

Note You need to log in before you can comment on or make changes to this bug.