GCLI needs a 'resource' type

RESOLVED FIXED in Firefox 13

Status

()

Firefox
Developer Tools
P2
normal
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jwalker, Assigned: jwalker)

Tracking

unspecified
Firefox 13
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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.
While we are at it, perhaps we should have a command to remove new / inject existing resources from / into a page.
Blocks: 711051
No longer blocks: 659052
Bug triage, filter on PEGASUS.
Target Milestone: --- → Firefox 12
Bug triage, filter on PEGASUS.
Priority: -- → P2
No longer blocks: 711051
Blocks: 683499
Created attachment 591752 [details] [diff] [review]
upload 1

a.k.a. https://github.com/campd/gcli/pull/19
Assignee: nobody → jwalker
Status: NEW → ASSIGNED
Attachment #591752 - Flags: review?(dcamp)
https://tbpl.mozilla.org/?tree=Try&rev=e6cb0ce0cbf3
Created attachment 592106 [details] [diff] [review]
upload 2

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

i.e. s/filter/include/
Attachment #591752 - Attachment is obsolete: true
Attachment #591752 - Flags: review?(dcamp)
Attachment #592106 - Flags: review?(dcamp)
Target Milestone: Firefox 12 → Firefox 13
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.
https://tbpl.mozilla.org/?tree=Try&rev=b447debd6053
Created attachment 593117 [details] [diff] [review]
upload 3

Change the name of the element to 'element'
Attachment #592106 - Attachment is obsolete: true
Attachment #592106 - Flags: review?(dcamp)
Attachment #593117 - Flags: review?(dcamp)

Comment 10

6 years ago
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?
Attachment #593117 - Flags: review?(dcamp) → feedback+
(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
https://tbpl.mozilla.org/?tree=Try&rev=946d867e53b4
https://tbpl.mozilla.org/?tree=Fx-Team&rev=3e9fbc7eb447
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/99d735b9cf8c
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
You need to log in before you can comment on or make changes to this bug.