Nicolas Chevobbe [:nchevobbe] Assignee
	Description • 6 years ago

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

	Nicolas Chevobbe [:nchevobbe] Assignee
	Comment 1 • 6 years ago

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).

	Nicolas Chevobbe [:nchevobbe] Assignee
	Comment 2 • 6 years ago

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.

	Nicolas Chevobbe [:nchevobbe] Assignee
	Comment 3 • 6 years ago

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

	Nicolas Chevobbe [:nchevobbe] Assignee
	Comment 4 • 6 years ago

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

	Pulsebot
	Comment 5 • 5 years ago

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.

	Nicolas Chevobbe [:nchevobbe] Assignee
	Comment 6 • 5 years ago

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.

	Pulsebot
	Comment 7 • 5 years ago

Backout by shindli@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6e8a03d07b12
Backed out changeset 3fa4474f947c as requested by nchevobbe

	Irene Smith
	Comment 8 • 5 years ago

(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.

	Nicolas Chevobbe [:nchevobbe] Assignee
	Comment 9 • 5 years ago

> 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!

	Irene Smith
	Comment 10 • 5 years ago

(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!

	Irene Smith
	Comment 11 • 5 years ago

Sorry, wrong page. This is the main page for the Web Console: https://developer.mozilla.org/en-US/docs/Tools/Web_Console

	Pulsebot
	Comment 12 • 5 years ago

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.