Allow to invoke getters in the autocomplete popup

RESOLVED FIXED in Firefox 66

Status

enhancement
P1
normal
RESOLVED FIXED
10 months ago
5 months ago

People

(Reporter: nchevobbe, Assigned: nchevobbe)

Tracking

(Blocks 1 bug, {dev-doc-needed})

unspecified
Firefox 66
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox66 fixed)

Details

(Whiteboard: [boogaloo-mvp])

User Story

Given the following object: 

```js
var customer = {
  get identity() {
    return {
      name: "Jane",
      age: 40,
    };
  }
};
```

In the console, if the user types `customer.identity.`, we don't provides the property on the returned object, since the `identity` getter might have side-effects (in the business logic, on the page, …).


What we could do though is allow the user to invoke said getter from the keyboard so they can access `name` or `age`.

Attachments

(3 attachments, 1 obsolete attachment)

Posted image mockups
Given the following object: 
```js
var customer = {
  get identity() {
    return {
      name: "Jane",
      age: 40,
    };
  }
};
```

In the console, if the user types `customer.identity.`, we don't provides the property on the returned object, since the `identity` getter might have side-effects (in the business logic, on the page, …).


What we could do though is allow the user to invoke said getter from the keyboard so they can access `name` or `age`.

---

Some mockups made by Matt are attached here. The "D" solution looks like a winner.

So if we get back to our example, on `customer.identity.`, a popup will be displayed with the following text: 


> +--------------------------+
> | Invoke getter *identity* |
> | to retrieve the property |
> | list ?                   |
> +--------------------------+
> | Confirm                  |
> +--------------------------+
> |              Learn more ?|
> +--------------------------+

Clicking on "Confirm" or hitting Enter would call a server function to execute the getter and return the list of properties for the returned object.

This should probably be possible to use something similar to the Object actor `propertyValue` function [1].

This could then be extended to functions as well (i.e. `customer.identity.name.trim().`)


[1] https://searchfox.org/mozilla-central/rev/3a54520d8d2319a4116866371ed3d9ed2ec0cc2b/devtools/server/actors/object.js#510-530
The `Learn more ?` item would be a link to an MDN page explaining why this is needed (i.e. getter execution can have side effects we - devtools - don't want to trigger automatically).
Whiteboard: [boogaloo-mvp]
Priority: -- → P2
Depends on: 1505393
Depends on: 1462394
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
Priority: P2 → P1
The functions needed to build the autocomplete request are moved
to the serviceContainer so it can be called from everywhere in
the code, and not only from the JsTerm.
This will be useful when implementing the invoke getter confirmation
dialog.
While trying to implement the invoke getter from
autocomplete popup, it became clear to me that the
initial implementation in js-property-provider wasn't
good enough, as we need to keep track of all the
authorizations the user gave when working on a given
expression.

In order to handle this, JsPropertyProvider now
returns an array of strings representing the path
to the unsafe getter when no matching authorizations
are provided.

If authorizations are provided, we can check for each
properties that the user authorized the execution.
This way, we can handle deep object access after a getter
(e.g. `x.myGetter.foo.bar.baz.entries`) without asking
the user if they want to invoke `myGetter` on each
step of the completion.

This makes handling intermediary getters (e.g.
`x.myGetter.foo.myOtherGetter.bar`) way easier as well.
In the UI, the user will be prompted to invoke the getter one
after the other (if for example they try to complete a pasted
expression which contains multiple getters, they will have
prompts for `myGetter`, and then for `myOtherGetter`).

We wire-up the webconsole client and the webconsole actor for
the autocomplete function, to make them ready for frontend
use.

The existing JsPropertyProvider getters test are updated to
match the change of parameter (invokeGetter -> authorizedEvaluations),
and some tests are added to make sure everything work as intended.

Depends on D12939
This patch introduces a new component, ConfirmDialog, that will
be rendered on screen when the autocomplete service indicates
that there's an unsafe getter in the completion path.
The component is rendered in the toolbox document, like the
autocomplete popup. In order to still write it in React, it uses
a React portal, which allow to render an element outside of the
React component tree.

In JsTerm, we still need to handle the key events, both for
codeMirror and the old JsTerm, in order to either call the
autocomplete service with the new computed authorizations, or
to hide the confirm dialog.

A test is added to make sure the dialog works as expected.

Depends on D12940
Depends on: 1513244
Attachment #9027593 - Attachment description: Bug 1499289 - Extract fetchAutocompleteProperties from JsTerm into a Redux action; r=Honza. → Bug 1513244 - Extract fetchAutocompleteProperties from JsTerm into a Redux action; r=Honza.
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3fa4474f947c
Bug 1513244 - Extract fetchAutocompleteProperties from JsTerm into a Redux action; r=Honza.
Irene, we'd like to have an MDN page that explains why we do ask for user authorization before trying to access a getter (because of possible side-effects).
Do you know who I should ping so I can create it? I don't seem to have the right to create a new page.
Flags: needinfo?(ismith)
Backout by shindli@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6e8a03d07b12
Backed out changeset 3fa4474f947c as requested by nchevobbe
Attachment #9027593 - Attachment is obsolete: true
Attachment #9027597 - Attachment description: Bug 1499289 - Allow to invoke getters from webconsole autocomplete function; r=bgrins. → Bug 1499289 - Allow to invoke getters from webconsole autocomplete function; r=bgrins!.
(In reply to Nicolas Chevobbe from comment #6)
> Irene, we'd like to have an MDN page that explains why we do ask for user
> authorization before trying to access a getter (because of possible
> side-effects).
> Do you know who I should ping so I can create it? I don't seem to have the
> right to create a new page.

I can help you. Tell me where it should go and I'd be happy to create it.

If you like, send me an outline of what you need and I will take care of the whole thing.
Flags: needinfo?(ismith)
> I can help you. Tell me where it should go and I'd be happy to create it.

So, under a webconsole page, if such page exists?
I'll try to work on a outline, thanks!
(In reply to Nicolas Chevobbe from comment #9)
> > I can help you. Tell me where it should go and I'd be happy to create it.
> 
> So, under a webconsole page, if such page exists?
> I'll try to work on a outline, thanks!

Yes, there is a console page: https://developer.mozilla.org/en-US/docs/Web/API/console

Ping me when you're ready!
Sorry, wrong page. This is the main page for the Web Console: https://developer.mozilla.org/en-US/docs/Tools/Web_Console
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9b92ad8d977d
Change how we deal with getter evaluation in JsPropertyProvider; r=bgrins.
https://hg.mozilla.org/integration/autoland/rev/bc5767b55411
Allow to invoke getters from webconsole autocomplete function; r=bgrins,flod.
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66
Blocks: 1518727
User Story: (updated)
Blocks: 1518733
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.