Closed Bug 1650658 Opened 1 year ago Closed 1 year ago

Can't jump to debugger tab from performance tab in call tree view

Categories

(DevTools :: Debugger, defect, P2)

79 Branch
defect

Tracking

(firefox-esr68 unaffected, firefox-esr78 wontfix, firefox78 wontfix, firefox79 verified, firefox80 verified)

VERIFIED FIXED
Firefox 80
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- wontfix
firefox78 --- wontfix
firefox79 --- verified
firefox80 --- verified

People

(Reporter: marvin, Assigned: daisuke)

References

(Regression)

Details

(Keywords: regression)

Attachments

(3 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:79.0) Gecko/20100101 Firefox/79.0

Steps to reproduce:

  1. Go to https://preactjs.com
  2. Open devtools
  3. Click on performance tab in devtools
  4. Click record
  5. Reload page
  6. Stop recording
  7. Click on Call Tree view and try to click on any js links

Actual results:

Nothing

Expected results:

Jump to the source location in the debugger like the title suggests

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Component: Untriaged → Debugger
Product: Firefox → DevTools

This was regressed by https://phabricator.services.mozilla.com/D68537 because it removed PerformanceController.toolbox but presently, that variable is used in 4 places (https://searchfox.org/mozilla-central/search?q=PerformanceController.toolbox&path=) to call PerformanceController.toolbox.viewSourceInDebugger(...), so now all of these places throw because toolbox is undefined.

@daisuke I can't quite tell why toolbox was removed, would that be easy to add back in?

Flags: needinfo?(daisuke)
Regressed by: 1603182

Thank you very much for reporting and I'm sorry for the inconvenience, marvin!
I could reproduce the issue.

Hi Logan, thank you very much for finding the cause!
Yes, it seems that it was caused by my patch.
I had thought that the toolbox is never referred from any places, I'm so sorry..
I'll fix this soon.

Flags: needinfo?(daisuke)
Assignee: nobody → daisuke

Thank you very much for reporting and I'm sorry for the inconvenience, marvin!

(...)

I had thought that the toolbox is never referred from any places, I'm so sorry.. I'll fix this soon.

@daisuke No worries, stuff like that happens to all of us and is a normal part of being a developer. Don't beat yourself up about it :)

(In reply to marvin from comment #6)

@daisuke No worries, stuff like that happens to all of us and is a normal part of being a developer. Don't beat yourself up about it :)
Thanks :)

Keywords: leave-open
Pushed by dakatsuka.birchill@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/93092fe1e43c
Revert PerformanceController to have toolbox. r=loganfsmyth
Severity: -- → S3
Priority: -- → P2
Attachment #9162551 - Attachment description: Bug 1650658 - Pass line and column position as numbers instead of strings. r=jlast! → Bug 1650658 - Pass line and column position as numbers instead of strings. r=davidwalsh!
Pushed by loganfsmyth@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f0e848b64e25
Pass line and column position as numbers instead of strings. r=davidwalsh
Regressed by: 1643180
Status: UNCONFIRMED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 80

@daisuke How do you feel about uplifting these patches? Seems like a good idea to me and unlikely to have any issues.

Flags: needinfo?(daisuke)

(In reply to Logan Smyth [:loganfsmyth] from comment #12)

@daisuke How do you feel about uplifting these patches? Seems like a good idea to me and unlikely to have any issues.

Hi @loganfsmyth!
Yes, I don't see any problems.
Could you do this since I am not sure the uplifting risk of your patch much?

Flags: needinfo?(daisuke) → needinfo?(loganfsmyth)

Comment on attachment 9162376 [details]
Bug 1650658: Revert PerformanceController to have toolbox.

Beta/Release Uplift Approval Request

  • User impact if declined: Users will be unable to click filenames in the Devtools Performance tool to open them in the debugger.
  • Is this code covered by automated tests?: No
  • 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 is not risky because it is reverting a change made in another patch by adding a property to an object that was incorrectly removed. The related patch in D82950 is also not risky because the only change is to parse a string value into a number value.
  • String changes made/needed:
Flags: needinfo?(loganfsmyth)
Attachment #9162376 - Flags: approval-mozilla-beta?
Attachment #9162551 - Flags: approval-mozilla-beta?

Comment on attachment 9162376 [details]
Bug 1650658: Revert PerformanceController to have toolbox.

Devtools regression fix. Approved for 79.0b9.

Attachment #9162376 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9162551 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

I’ve reproduced this issue with Fx 80.0a1 (2020-07-05) on Windows 10.
The issue is fixed with Fx 80.0a1 (2020-07-19) and Fx 79.0b9 on Windows 10 and Ubuntu 18.04.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.