1590824 - Jump to definition button jumps to buggy view-source: link

Status: VERIFIED FIXED

(DevTools :: Console, defect, P2)

Description

[mattheww]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:72.0) Gecko/20100101 Firefox/72.0

Steps to reproduce:

1. Go to https://firefox-dev.tools/debugger-examples/examples/pythagorean/
2. Open pythagorean.js and pretty-print it
3. Type math in the console
4. Click the jump-to-definition button

Actual results:

You'll be taken to a new view-source: tab showing the page source.

Expected results:

You should be taken to the pretty-printed definition of math in the debugger.

Comment 1

[:Harald Kirschner :digitarald]

(aside, love how we matched our bug titles so well)

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

I can't reproduce for some reason?

Comment 3

[:Harald Kirschner :digitarald]

Reproduced on https://debugger-pretty-print-events-1532240.glitch.me/ with

1. pretty print compressed.js
2. log setup
3. jump to setup definition

Probably same as bug 1589346.

Comment 4

[mattheww]

It is the same, yeah. I'm sorry for the bad steps to reproduce it. I'm guessing I should fix the bug the same way, by not passing sourceId?

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

(In reply to mattheww from comment #4)

It is the same, yeah. I'm sorry for the bad steps to reproduce it. I'm guessing I should fix the bug the same way, by not passing sourceId?

yeah probably.

Comment 6

[mattheww]

Attachment: Bug 1590824 - Don't pass sourceId in jumpToDefinitionButton event listener. r=nchevobbe.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/df999d1f7c6e
Don't pass sourceId in jumpToDefinitionButton event listener. r=nchevobbe.

Comment 8

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/df999d1f7c6e

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Is this something we should consider uplifting to Beta or can this ride Fx72 to release?

Comment 10

[Nicolas Chevobbe [:nchevobbe]]

yes, I'll request the uplift right away

Comment 11

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 9104770 [details]
Bug 1590824 - Don't pass sourceId in jumpToDefinitionButton event listener. r=nchevobbe.

Beta/Release Uplift Approval Request

• User impact if declined: Clicking on a "jump to definition" icon in the console might not navigate the user to the debugger, but open a weird, blank tab.
  This is a regression and the fix is similar to the one of Bug 1433373, that was already uplifted.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See Comment 3
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Small devtools only change, only affecting logged functions whose source have been prettified.
• String changes made/needed:

Comment 12

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9104770 [details]
Bug 1590824 - Don't pass sourceId in jumpToDefinitionButton event listener. r=nchevobbe.

Fix for a 71 regression in devtools, low risk of causing worse regressions, uplift approved for 71 beta 8, thanks.

Comment 13

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d09fb9cdaf50

Comment 14

[Daniel Bodea [:danibodea]]

I am attempting to verify the fix of this bug, but I do not understand the steps to reproduce. Can you help enlighten my confusion? Keep in mind that the technical level should be low enough that pretty much anyone should be able to reproduce it using the steps provided.

Thank you for your contribution!

Comment 15

[mattheww]

I'm really sorry about that, I know my steps in the bug report were not great.

The steps are pretty much the same as for bug 1589346, but using a function logged to the console:

1. Open the console on https://debugger-pretty-print-events-1532240.glitch.me/ (ctrl-shift-K on Windows/Linux, cmd-option-K on Mac)
2. Enter setup in the console
3. The console will reply with function setup() and a jump-to-definition button. Clicking it should take you to line 17 of compressed.js
4. Pretty-print compressed.js ({} icon in the lower left)
5. Go back to the console
6. Click the jump-to-definition button again. It should take you to line 150787 of the pretty-printed compressed.js.
   Versions with the regression will instead open a new tab with a view-source: URL and pop up a "The specified line was not found" message.

I hope this helps, thank you for verifying!

Comment 16

[Daniel Bodea [:danibodea]]

Using the steps provided, I have reproduced the issue in Nightly v72.0a1 from 2019-10-23 and in Beta v71.0b7.
I have verified this fix in Nightly v72.0a1 from 2019-11-07 and in Beta Candidate v71.0b8 in Windows 10 and Mac OS 10.14.6.