Closed Bug 1552609 Opened 5 months ago Closed 5 months ago

Adding a logpoint fails and breaks locations

Categories

(DevTools :: Debugger, defect, P1, critical)

defect

Tracking

(firefox-esr60 unaffected, firefox67 wontfix, firefox68 fixed, firefox69 fixed)

RESOLVED FIXED
Firefox 69
Tracking Status
firefox-esr60 --- unaffected
firefox67 --- wontfix
firefox68 --- fixed
firefox69 --- fixed

People

(Reporter: Harald, Assigned: jlast)

References

(Blocks 1 open bug)

Details

(Keywords: regression, Whiteboard: [debugger-mvp])

Attachments

(4 files, 1 obsolete file)

Attached video Flow on Perfetto

What were you doing?

(see video)

  1. Inspected event location in https://ui.perfetto.dev/#!/viewer
  2. Set a Breakpoint somewhere in https://ui.perfetto.dev/frontend_bundle.js
  3. Set a logpoint on the BP

What happened?

Debugger jumps and afterwards does not show any added breakpoints in the gutter.

After the STR, breakpoints are broken throughout Debugger, any file, until it gets closed and re-opened.

After the STR, jumping to sources from the events bubble is also broken and jumps to the wrong line.

What should have happened?

Logpoints and Debugger work.

Priority: -- → P2

P1 until we have more insights into this.

Logan, could this be related to the column bp work?

Flags: needinfo?(lsmyth)
Priority: P2 → P1

I started looking into this and noticed that the original file track_panel.ts works fine, ...

okay - so what was happening was that we were showing the conditional panel at the original line (177) and not letting you do anything while it was open...

this is a pretty easy fix

Assignee: nobody → jlaster
Status: NEW → ASSIGNED
Whiteboard: [debugger-mvp]
Attachment #9066212 - Attachment is obsolete: true
Flags: needinfo?(lsmyth)
Pushed by jlaster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b6d6421ec910
Adding a logpoint fails and breaks locations. r=davidwalsh
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 69

Please request uplift to beta if and when you feel comfortable doing so.

Flags: needinfo?(jlaster)
Flags: in-testsuite+
Attached patch beta-logs.patchSplinter Review

Beta/Release Uplift Approval Request

  • User impact if declined: breakpoints can't be edited in sourcemapped files
  • 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): it only affects the debugger
  • String changes made/needed:
Flags: needinfo?(jlaster)
Attachment #9067036 - Flags: approval-mozilla-beta?
Attachment #9066188 - Flags: approval-mozilla-beta?

hmm, i'm a bit confused my new patch is on top of beta, but the old would probably apply too.

Comment on attachment 9066188 [details]
Bug 1552609 - Adding a logpoint fails and breaks locations. r=davidwalsh

debugger fix, approved for 68.0b5

Attachment #9066188 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 9067036 [details] [diff] [review]
beta-logs.patch

Grafting the patch from central should be fine, I think.
Attachment #9067036 - Flags: approval-mozilla-beta?

Is this something you would like to see uplifted to release?

Flags: needinfo?(jlaster)

I don't think this is necessary. I have a pretty high threshold for uplifting to release.

Flags: needinfo?(jlaster)
You need to log in before you can comment on or make changes to this bug.