1398729 - Support devtools.panels.elements sidebar.setExpression API method

Status: VERIFIED FIXED

(WebExtensions :: Developer Tools, defect, P2)

Description

[Luca Greco [:rpl] [:luca] [:lgreco]]

As a follow up to Bug 1341305, which has introduced the devtools.panels.elements.createSidebarPane and an initial subset of the APIs provided by the resolved sidebar API object (sidebar.onShown/onHidden and sidebar.setObject), the goal of this issue is to provide the sidebar.setExpression API method, which populate the sidebar's object TreeView with the result of the evaluation of the given expression.

Comment 1

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1398729 - Share devtools.inspectedWindow.eval helper methods in ext-devtools API module.

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

Comment 2

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1398729 - Support devtools.panels.elements sidebar.setExpression API method.

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

Comment 3

[Andrew Swan [:aswan]]

Comment on attachment 8906570 [details]
Bug 1398729 - Share devtools.inspectedWindow.eval helper methods in ext-devtools API module.

https://reviewboard.mozilla.org/r/178316/#review183502

Just FYI, I think Kris is trying to avoid using import-globals-from eslint comments but I don't know eactly what his plans are and this matches the existing style.

Comment 4

[Andrew Swan [:aswan]]

Comment on attachment 8906571 [details]
Bug 1398729 - Support devtools.panels.elements sidebar.setExpression API method.

https://reviewboard.mozilla.org/r/178318/#review183506

::: browser/components/extensions/ext-devtools-panels.js:379
(Diff revision 1)
>  
>  this.devtools_panels = class extends ExtensionAPI {
>    getAPI(context) {
> +    // Lazily retrieved inspectedWindow actor front per child context
> +    // (used by Sidebar.setExpression).
> +    let waitForInspectedWindowFront;

Why is this a promise and not the actual thing the promise resolves to?

Comment 5

[Luca Greco [:rpl] [:luca] [:lgreco]]

Comment on attachment 8906571 [details]
Bug 1398729 - Support devtools.panels.elements sidebar.setExpression API method.

https://reviewboard.mozilla.org/r/178318/#review183506

> Why is this a promise and not the actual thing the promise resolves to?

we save the returned promise instead of the resolved value to prevent a race ( https://bugzilla.mozilla.org/show_bug.cgi?id=1300584#c51 ).

Comment 6

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Andrew Swan [:aswan] from comment #3)
> Comment on attachment 8906570 [details]
> Bug 1398729 - Share devtools.inspectedWindow.eval helper methods in
> ext-devtools API module.
> 
> https://reviewboard.mozilla.org/r/178316/#review183502
> 
> Just FYI, I think Kris is trying to avoid using import-globals-from eslint
> comments but I don't know eactly what his plans are and this matches the
> existing style.

Oh yeah you are right (and thanks for pointing it out)

I've been following the discussion on the related bugzilla issue (Bug 1372406) and I thought that in the meantime it is better to keep it consistent to the current style so that we can more easily migrate them to the new convention all at once.

Comment 7

[Luca Greco [:rpl] [:luca] [:lgreco]]

Comment on attachment 8906570 [details]
Bug 1398729 - Share devtools.inspectedWindow.eval helper methods in ext-devtools API module.

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

Comment 8

[Luca Greco [:rpl] [:luca] [:lgreco]]

Comment on attachment 8906571 [details]
Bug 1398729 - Support devtools.panels.elements sidebar.setExpression API method.

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

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d3df8bc53dcb
Share devtools.inspectedWindow.eval helper methods in ext-devtools API module. r=aswan
https://hg.mozilla.org/integration/autoland/rev/7ef7bcbd8884
Support devtools.panels.elements sidebar.setExpression API method. r=aswan

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d3df8bc53dcb
https://hg.mozilla.org/mozilla-central/rev/7ef7bcbd8884

Comment 11

[Will Bamberg [:wbamberg]]

Docs notes are in bug 1341305, comment 29.

Comment 12

[CosminB]

Attachment: Bug1398729.zip

I can reproduce this issue on Firefox 56.0.2 (20171024165158) under Wind 7 64-bit.

The next error is displayed in the browser console:
TypeError: chrome.devtools.panels.elements.createSidebarPane is not a function  devtools_page.js:1:1

This issue is verified as fixed on Firefox 59.0a1 (20171128100440), Firefox 58.0b7 (20171127135700) and Firefox 57.0 (20171112125346) under Win 7 64-bit and Mac OS X 10.13.1

Please see the attached screenshots.