Closed Bug 1515046 Opened 6 years ago Closed 6 years ago

Console runs the wrong getter if it's shadowed

Categories

(DevTools :: Console, defect, P2)

Product:
DevTools
Component:
Console
Version:
65 Branch
Type:
defect

Tracking

(firefox-esr60 unaffected, firefox64 unaffected, firefox65 verified, firefox66 verified)

Status:
VERIFIED FIXED
Milestone:
Firefox 66
Tracking Flags:
Tracking Status
firefox-esr60 --- unaffected
firefox64 --- unaffected
firefox65 --- verified
firefox66 --- verified

People

(Reporter: Oriol, Assigned: Oriol)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

Bug 1515046 - Add receiverId parameter in objectClient.getPropertyValue. r=nchevobbe
(deleted), text/x-phabricator-request
Details
Bug 1515046 - Let reps invoke the right getter when it's shadowed. r=nchevobbe
(deleted), text/x-phabricator-request
RyanVM
: approval-mozilla-beta+
Details
Bug 1513505 made inherited getters to be invoked in the root object instead of the owner one. However, it introduced a regression: ({ getter: 2, __proto__: { get getter() {return 1} } }) Expand <prototype> and run the getter. Expected result: 1 Actual result: 2 Or a more comprehensive test: ({ value: 2, get getter() { return this.value * 3 }, __proto__: { value: 5, get getter() { return this.value * 7 }, } }) Expand <prototype> and run the getter. Expected result: 14 Actual result: 6 That is, the getter that is evaluated should still be the one chosen by the user. It's just that the receiver should be the root object.
Flags: needinfo?(nchevobbe)
Depends on: 1515447
Flags: needinfo?(nchevobbe)
Priority: -- → P2
Assignee: nobody → oriol-bugzilla
Attachment #9033892 - Attachment is obsolete: true

Pushed by ncsoregi@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/874c30e2b934
Add receiverId parameter in objectClient.getPropertyValue. r=nchevobbe
https://hg.mozilla.org/integration/autoland/rev/524ec48474fd
Let reps invoke the right getter when it's shadowed. r=nchevobbe

Keywords: checkin-needed

https://hg.mozilla.org/mozilla-central/rev/874c30e2b934
https://hg.mozilla.org/mozilla-central/rev/524ec48474fd

Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox66: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66

Please nominate this for Beta approval when you get a chance.

Flags: needinfo?(oriol-bugzilla)
Flags: in-testsuite+

Comment on attachment 9034581 [details]
Bug 1515046 - Let reps invoke the right getter when it's shadowed. r=nchevobbe

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1513505

User impact if declined: Console invokes the wrong getter if the one clicked by the user is shadowed

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: Yes

If yes, steps to reproduce: 1. Open the console
2. Evaluate the following code:

({
  value: 2,
  get getter() { return this.value * 3 },
  __proto__: {
    value: 5,
    get getter() { return this.value * 7 },
  }
})
  1. Expand the object
  2. Expand <prototype>
  3. Click '>>' next to 'getter' to invoke the getter
  4. Expected result: 14

List of other uplifts needed: Bug 1515046, Bug 1515447

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): This bug only has devtools changes for getter invocation, and are covered by tests.
Bug 1515447 has JS changes in DebuggerObject but they mimic existing code and are also covered by tests.

String changes made/needed:

Flags: needinfo?(oriol-bugzilla)
Attachment #9034581 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Needs a rebased patch for Beta uplift.

Flags: needinfo?(oriol-bugzilla)

I can apply the patches cleanly to latest beta 5b52f9f83004.
Note that the order should be 1st D15018, 2nd D15788, 3rd D15789.
D15789 does not apply cleanly without D15788.

Flags: needinfo?(oriol-bugzilla)

Oh, I missed the dependency on the other bug. It also needs an uplift request.

Flags: needinfo?(oriol-bugzilla)

OK I thought a single uplift request sufficed. But you should have been able to apply the patches from this bug without the other one (the tests would fail, though)

Flags: needinfo?(oriol-bugzilla)

Fun, I can import D15788 from Phabricator without issue, but grafting 874c30e2b934 has conflicts in reps.js.

Comment on attachment 9034581 [details]
Bug 1515046 - Let reps invoke the right getter when it's shadowed. r=nchevobbe

[Triage Comment]
Fixes a devtools bug. Approved for 65.0b11.

Attachment #9034581 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

I have reproduced this issue using Firefox 66.0a1 (2018.12.18) on Win 10 x64.
I can confirm this issue is fixed, I verified using Firefox Nightly 66.0a1 and Firefox 65.0b11 build from Taskcluster on Win 10 x64, Mac OS X 10.13.6 and Ubuntu 18.04 x64.

Status: RESOLVED → VERIFIED
status-firefox65: fixed → verified
status-firefox66: fixed → verified
Flags: qe-verify+

Thanks a lot Oriol for fixing this + requesting the uplift!

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: