Closed Bug 943496 Opened 10 years ago Closed 10 years ago

Autocomplete should execute native getters


(DevTools :: Console, defect)

Not set


(Not tracked)

Firefox 28


(Reporter: msucan, Assigned: msucan)



(Keywords: regression)


(2 files, 1 obsolete file)

document.title returns a getter and completion suggestions are inaccurate. We need to execute safe/native getters.
Regression caused by bug 842682. We have a test that was supposed to check this stuff, browser_webconsole_bug_651501_document_body_autocomplete.js. However, it changed too much in time and now it's too permissive - it doesn't check which properties are shown in the popup.
Depends on: 842682
Keywords: regression
Attached patch bug-943496-1-fx-team.diff (obsolete) — Splinter Review
Patch started as a bit of code shuffle in JSPropertyProvider - it grew too confusing / easy to break. Then I found the problem with executing native getters - see DebuggerObjectSupport.getProperty().

Any improvements we could make?

In the future, it would be nice if we could reuse stuff from actors/script.js. I saw we have functions there for similar purposes.

(no try push yet, due to tree closure)
Attachment #8338831 - Flags: review?(past)
Comment on attachment 8338831 [details] [diff] [review]

Review of attachment 8338831 [details] [diff] [review]:

Looks fine to me, but it would be better to add an explicit test for document.title or a similar getter.

::: toolkit/devtools/webconsole/utils.js
@@ +1037,5 @@
> +    let safe = desc.get.callable && desc.get.class == "Function" &&
> +               desc.get.script === undefined;
> +    if (!safe) {
> +      return null;

Maybe we should move the hasSafeGetter() and getProperty() helper functions from script.js to DevToolsUtils.js and reuse them here, but it's up to you.
Attachment #8338831 - Flags: review?(past) → review+
Thanks for the review!

Updated the patch to move getProperty() and hasSafeGetter() as suggested. Also added testing for document.title.

Do note that I fixed two issues in getProperty(): (1) it called native getter functions on prototype objects, not on the root/start object, which made it fail in many cases; (2) when it executed a native getter it returned a debugger completion value, not the actual function return value - breaking its callers. I found these problems when working on bug 843004.

Try push:
Attachment #8338831 - Attachment is obsolete: true
bonus interdiff :)
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 28
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.