Closed Bug 1530549 Opened 6 months ago Closed 6 months ago

Debugger steps over when I click "Step out"

Categories

(DevTools :: Debugger, defect)

65 Branch
defect
Not set

Tracking

(firefox-esr60 unaffected, firefox65 wontfix, firefox66 fixed, firefox67 fixed)

RESOLVED FIXED
Firefox 67
Tracking Status
firefox-esr60 --- unaffected
firefox65 --- wontfix
firefox66 --- fixed
firefox67 --- fixed

People

(Reporter: Oriol, Assigned: loganfsmyth)

References

Details

(Keywords: regression)

Attachments

(2 files)

  1. Run this code in the console:
(function() {
  (function(){debugger;})();
  var a = 1;
  a = 2;
  a = 3;
  a = 4;
})()
  1. The debugger pauses at debugger.
  2. Click "Step out", the debugger steps out and pauses at var a = 1
  3. Click "Step out" again and again. The debugger steps over instead of exiting the function.

Regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=dc1eb225488c72c73dabcce4179b94e3dd008ca9&tochange=f668df79d93b379add8c1019792e6fdef687506b

I suspect bug 1508314

Flags: needinfo?(bhackett1024)

You are correct about which bug introduced this, patch incoming.

Flags: needinfo?(bhackett1024)
Assignee: nobody → lsmyth
Pushed by lsmyth@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6579b451a352
Ensure that _clearSteppingHooks() runs _after_ we pause. r=jlast
Status: NEW → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67

Please request uplift to beta when you get a chance.

Has Regression Range: --- → yes
Has STR: --- → yes
Keywords: regression

Comment on attachment 9047095 [details]
Bug 1530549 - Ensure that _clearSteppingHooks() runs after we pause. r=jlast!

Beta/Release Uplift Approval Request

  • Feature/Bug causing the regression: Bug 1508314
  • User impact if declined: JS debugger's stepOut stepping feature will behave in unexpected ways that can be frustrating for users
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This change can only affect the call to this_clearSteppingHooks() since that is the only function call that happens between the new and old _state initialization location, and that is specifically the case that we are fixing by moving this line.
  • String changes made/needed:
Attachment #9047095 - Flags: approval-mozilla-beta?

Comment on attachment 9047095 [details]
Bug 1530549 - Ensure that _clearSteppingHooks() runs after we pause. r=jlast!

Fix limited to debugger, OK for beta uplift.

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

There are conflicts while uplifting to beta.
warning: conflicts while merging devtools/server/actors/thread.js! (edit, then use 'hg resolve --mark')
warning: conflicts while merging devtools/server/tests/unit/xpcshell.ini! (edit, then use 'hg resolve --mark')

:loganfsmyth , can you please take a look?

Flags: needinfo?(lsmyth)
Attached patch bug1530549.patchSplinter Review

Here is a new patch that won't conflict on 66, sorry about that.

Should I mark this one for beta now, or is the approval on the other one acceptable?

Flags: needinfo?(lsmyth) → needinfo?(nbeleuzu)
Flags: needinfo?(nbeleuzu)
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.