1353006 - Stop using Add-on SDK modules in JSONViewer

Status: RESOLVED FIXED

(DevTools :: JSON Viewer, enhancement, P1)

Description

[Patrick Brosset <:pbro>]

In bug 1350645 we're trying to stop using certain SDK APIs, including sdk/dom/events, which is being used in JSONView:

http://searchfox.org/mozilla-central/rev/990055a4902952e038cc350c9ffe1ac426d1c943/devtools/client/jsonview/converter-child.js#15-16

In this bug, we should find an alternative to this API.

Comment 1

[Patrick Brosset <:pbro>]

Honza: is this something you could find a solution for? Is there another way we could deal with these events that did not have to rely on this SDK API, nor any of the other SDK APIs we want to remove in bug 1350645 comment 2?

Comment 2

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: Bug 1353006 - Stop using sdk/dom/events in JSONViewer;

Review commit: https://reviewboard.mozilla.org/r/126778/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/126778/

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Patrick Brosset <:pbro> from comment #1)
> Honza: is this something you could find a solution for? Is there another way
> we could deal with these events that did not have to rely on this SDK API,
> nor any of the other SDK APIs we want to remove in bug 1350645 comment 2?
Patch removing sdk/events attached.

Not sure about "sdk/clipboard", I'll look into it yet.

Honza

Comment 4

[Patrick Brosset <:pbro>]

Comment on attachment 8854812 [details]
Bug 1353006 - Stop using sdk/dom/events in JSONViewer;

https://reviewboard.mozilla.org/r/126778/#review129400

Thank you Honza!

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: Bug 1353006 - Stop using sdk/clipboard in JSONViewer;

Review commit: https://reviewboard.mozilla.org/r/126794/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/126794/

Comment 6

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: Bug 1353006 - Stop using sdk/window/utils in JSONViewer;

Review commit: https://reviewboard.mozilla.org/r/126796/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/126796/

Comment 7

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8854812 [details]
Bug 1353006 - Stop using sdk/dom/events in JSONViewer;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/126778/diff/1-2/

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8854840 [details]
Bug 1353006 - Stop using sdk/clipboard in JSONViewer;

https://reviewboard.mozilla.org/r/126794/#review129406

::: devtools/client/jsonview/converter-child.js:321
(Diff revision 1)
> +  let doCopy = function (e) {
> +    e.clipboardData.setData("text/plain", string);
> +    e.preventDefault();
> +  };
> +
> +  win.document.addEventListener("copy", doCopy);

What about using a `once: true` listener here?  (I know you just copied this, but still...) :)

Comment 9

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8854841 [details]
Bug 1353006 - Stop using sdk/window/utils in JSONViewer;

https://reviewboard.mozilla.org/r/126796/#review129408

::: devtools/client/jsonview/utils.js:40
(Diff revision 1)
>  
>      filePickerShown = true;
>  
>      let fp = Cc["@mozilla.org/filepicker;1"].createInstance(Ci.nsIFilePicker);
>  
> -    let win = getMostRecentBrowserWindow();
> +    let win = WM.getMostRecentWindow("navigator:browser");

`Services.wm.<...>`, no need for `getService`.

Comment 10

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8854840 [details]
Bug 1353006 - Stop using sdk/clipboard in JSONViewer;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/126794/diff/1-2/

Comment 11

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8854841 [details]
Bug 1353006 - Stop using sdk/window/utils in JSONViewer;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/126796/diff/1-2/

Comment 12

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8854841 [details]
Bug 1353006 - Stop using sdk/window/utils in JSONViewer;

https://reviewboard.mozilla.org/r/126796/#review129446

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/62f1e7e410ce
Stop using sdk/dom/events in JSONViewer; r=pbro
https://hg.mozilla.org/integration/autoland/rev/38cfa259d025
Stop using sdk/clipboard in JSONViewer; r=jryans
https://hg.mozilla.org/integration/autoland/rev/2385f24df5c6
Stop using sdk/window/utils in JSONViewer; r=jryans

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/62f1e7e410ce
https://hg.mozilla.org/mozilla-central/rev/38cfa259d025
https://hg.mozilla.org/mozilla-central/rev/2385f24df5c6