Closed Bug 1557138 Opened 5 months ago Closed 3 months ago

Click logpoint location in console will open logpoint panel in Debugger

Categories

(DevTools :: Console, enhancement, P3)

69 Branch
enhancement

Tracking

(firefox70 fixed)

RESOLVED FIXED
Firefox 70
Tracking Status
firefox70 --- fixed

People

(Reporter: chujunlu, Assigned: chujunlu)

References

Details

Attachments

(2 files)

Attached image 1mLKyxTrnO.gif

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/74.0.3729.169 Safari/537.36

Steps to reproduce:

  1. Open link https://luxuriant-system.glitch.me/
  2. Open Debugger
  3. At script.js line 3, add a logpoint, expression: b is ${b}
  4. In console, run: loopThrough([0]);
  5. A message with the logpoint's location is printed in the console
  6. Click script.js:3:15

Actual results:

Jumps to debugger, highlights the line in editor for a second; doesn't show anything about the log point expression.

Expected results:

Jump to the debugger and open the log point editing panel.

Thanks for filing, this is an interesting idea that we should explore as it makes iterating on logs easier. Marking this as en enhancement, as it changes how the feature works.

Is the logpoint editing behavior something you have seen in other tools?

Type: defect → enhancement
Flags: needinfo?(chujunlu)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3

(In reply to :Harald Kirschner :digitarald from comment #1)

Thanks for filing, this is an interesting idea that we should explore as it makes iterating on logs easier. Marking this as en enhancement, as it changes how the feature works.

Is the logpoint editing behavior something you have seen in other tools?

Actually, No. Chrome's debugger is the only other debugger I'm familiar with. Chrome doesn't throw a console message if the log point expression is invalid. It ignores that log point.

The idea of this new feature came up in my patch https://phabricator.services.mozilla.com/D31157 for https://bugzilla.mozilla.org/show_bug.cgi?id=1536854 . I think we need a new bug for more discussion. So here it comes.

Flags: needinfo?(chujunlu)

Now I understand; for errors caused by logpoints it makes sense to open the expression by default.

I'd say even for regular logpoints.
Let's say you added a bunch of them in your code, and the console now has several messages coming from different places. You might not remember which log represent which data/property (especially if you get null, undefined, false, …)
When you have a console.log, you click on the location, are taken to debugger and you see what you are logging (e.g. console.log(name)).
With a logpoint, at the moment, you have no clue. It goes to the debugger at the line you added the logpoint, but you still can't know what was logged, and probably have to do another click to reveal the text input.
The proposed feature here would save you a click.
Plus, I can think of cases where the logpoint you entered doesn't output enough data to debug your issue, so you want to edit it. Having this feature make that faster.
I don't think there's much drawback to this.
We could have different targets in the location (the Logpoint at part would open the logpoint input, the "real" location would only jump to the debugger), but I'm not sure that would be discoverable, and it might confuse people.

I don't think there's much drawback to this.

Agreed, especially since the logpoint UI is 2 clicks away but is easy to close by de-focusing.

Sounds good. I'm interested in implementing it :)

Chujun, should I assign the bug to you?

Flags: needinfo?(chujunlu)

Yes, that would be great. Thanks!

Flags: needinfo?(chujunlu)

Here we go! Thanks a lot for working on this!

Assignee: nobody → chujunlu
Status: NEW → ASSIGNED

Hey @nchevobbe, I have a working implementation but I'm struggling with writing a Mochitest for the behavior. May I submit a WIP patch and ask questions? Thanks!

(In reply to Chujun Lu from comment #10)

Hey @nchevobbe, I have a working implementation but I'm struggling with writing a Mochitest for the behavior. May I submit a WIP patch and ask questions? Thanks!

Sure, push to phabricator and we can discuss how things shoud look :) Thanks!

I pass frame.origin in viewSourceInDebugger. When reason is logpoint, run openConditionalPanel.

For posterity: We discussed the UX flow in https://github.com/firefox-devtools/ux/issues/79#issuecomment-513005696 and agreed to move forward.

(In reply to :Harald Kirschner :digitarald from comment #13)

For posterity: We discussed the UX flow in https://github.com/firefox-devtools/ux/issues/79#issuecomment-513005696 and agreed to move forward.

Thank you for the UX discussion. If I get it right, the decision for now is to keep this feature - open editing panel. Should I reactivate the working patch by adding back @nchevobbe and Jason as reviewers? Thanks all for your patience.

Blocks: 1573890
Pushed by jlaster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6e86ec2e3a21
Clicking logpoint location in console opens the logpoint editing panel in debugger r=nchevobbe,jlast
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 70
QA Whiteboard: [good first verify]
You need to log in before you can comment on or make changes to this bug.