Add button to load memory reports from the clipboard

RESOLVED FIXED in mozilla18

Status

()

Toolkit
about:memory
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Justin Lebar (not reading bugmail), Assigned: Justin Lebar (not reading bugmail))

Tracking

Trunk
mozilla18
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
This is pretty useful to me in practice!
(Assignee)

Comment 1

5 years ago
Created attachment 668009 [details] [diff] [review]
Patch, v1
Attachment #668009 - Flags: review?(n.nethercote)
(Assignee)

Updated

5 years ago
Assignee: nobody → justin.lebar+bug
(Assignee)

Comment 2

5 years ago
I didn't add a test because it seems like way more effort than it's worth (particularly because we're re-using most of the existing show-from-files code), but I'm happy to figure out a test if you think it's worthwhile.
(Assignee)

Updated

5 years ago
Depends on: 768470
Comment on attachment 668009 [details] [diff] [review]
Patch, v1

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

I agree a test isn't necessary.

What's the specific use case, BTW?  I'm having trouble imagining it.

::: toolkit/components/aboutmemory/content/aboutMemory.js
@@ +443,4 @@
>      reader.readAsText(aFile);
>  
>    } catch (ex) {
> +    let body = clearBody();

Would it be better to move this to the top of the try block (where it was) and then pass |body| to updateAboutMemoryFromJSONString()?  Not sure, I'll let you decide.

@@ +457,5 @@
> +{
> +  // Get the clipboard's contents.
> +  let cb = Cc["@mozilla.org/widget/clipboard;1"]
> +             .getService(Components.interfaces.nsIClipboard);
> +  let xferable = Cc["@mozilla.org/widget/transferable;1"]

|xferable| strikes me as a very un-jlebar-ish variable name, but I'll allow it :)
Attachment #668009 - Flags: review?(n.nethercote) → review+
(Assignee)

Comment 4

5 years ago
> What's the specific use case, BTW?  I'm having trouble imagining it.

e.g. cjones posts a memory report dump in bug 797189.  I can very quickly open it up, copy it to my clipboard, and click the button in about:memory.  Saving it to a file and then picking the file is much slower.

> |xferable| strikes me as a very un-jlebar-ish variable name, but I'll allow it :)

Wow, was it that obvious I copied this code??
Is the input here properly sanitized for realz? The easier we make it to input random stuff into about:memory the closer we are to some kind of XSS thing. Now, admittedly this still requires the user cutting and pasting, but it is better to be cautious.
> Is the input here properly sanitized for realz?

Yes:  it goes through JSON.parse.
(Assignee)

Comment 7

5 years ago
(In reply to Nicholas Nethercote [:njn] from comment #6)
> > Is the input here properly sanitized for realz?
> 
> Yes:  it goes through JSON.parse.

And we never set innerHTML, so we're good in theory.

It strikes me that for the use-case I'm interested in, being able to specify a URL to fetch the about:memory dump from would be particularly convenient.  That way, we could fetch a compressed dump, whereas right now, if I want to be able to use this convenient copy/paste thing, I have to post uncompressed dumps to bugzilla, which are obscenely large.  But that's for a separate bug.
(Assignee)

Comment 8

5 years ago
> Would it be better to move this to the top of the try block (where it was) and then pass |body| to 
> updateAboutMemoryFromJSONString()?  Not sure, I'll let you decide.

I was on the fence about this, but I think it's maybe better how it is because functions with fewer arguments are better, and functions with fewer assumptions are better.  (Otherwise updateAboutMemoryFromJSONString would assume that the |body| it got had just been cleared.)

Comment 9

5 years ago
https://hg.mozilla.org/mozilla-central/rev/aa211d740d88
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.