Opening conditional panel with keyboard shortcut should display previously saved condition
Categories
(DevTools :: Debugger, defect, P2)
Tracking
(firefox71 fixed)
Tracking | Status | |
---|---|---|
firefox71 | --- | fixed |
People
(Reporter: anthonyxie64, Assigned: arosenfeld2003, NeedInfo)
References
(Blocks 2 open bugs)
Details
(Keywords: good-first-bug, Whiteboard: [debugger-reserve])
Attachments
(2 files)
Thank you for helping make Firefox better. If you are reporting a defect, please complete the following:
What were you doing?
- Open debugger on a page and right click on a gutter line number
- Popup menu should appear, click "add condition"
- Conditional panel should open, add a condition
- Press Enter to save conditional breakpoint, conditional panel should close
- Click on the line with conditional breakpoint in the editor
- Now use the keyboard shortcut to open the conditional panel on that line (Ctrl+Shift+B or Cmd+Shift+B)
What happened?
Actual Result: Conditional panel opens without previously saved condition
What should have happened?
Expected Result: Conditional panel opens populated with previously saved condition
Anything else we should know?
If a conditional breakpoint is created using the keyboard shortcut, we do not see this problem
Updated•6 years ago
|
Updated•6 years ago
|
Comment 1•6 years ago
|
||
Since we now use column breakpoints, this requires a bit more thought. A few things to think about:
-
The gutter marker represents the first activated or potential breakpoint in the line.
-
What do we do if there's an active column breakpoint and another existing conditional breakpoint on the line?
Hi David and Anthony. I've actually been working on this and came up with a potential solution to the original bug:
It involves a couple changes to src/components/Editor/ConditionalPanel.js
:
-
the function
getBreakpointForLocation
returns only the location, while we want the breakpoint object from state (which includes the condition). -
importing and replacing this with the similarly named function
getBreakpointAtLocation
fromsrc/selectors/breakpointAtLocation.js
solves the problem.
Here's a demo:
http://g.recordit.co/pXQd2RIzdC.gif
This causes a flow error which I am still investigating:
Cannot call `getBreakpointAtLocation` with `location` bound to `location`
because null [1] is incompatible with `LineColumn` [2].Flow(InferError)
ui.js(177, 4): [1] null
breakpointAtLocation.js(66, 65): [2] `LineColumn`
I was also thinking about the case of additional column breakpoints...
Currently (as you stated above) my solution always opens the gutter breakpoint (first breakpoint on the line).
Would it be preferable for the shortcut to open the column breakpoint closest to where the user has clicked?
Or, maybe we want to display all the column breakpoints on the line and allow the user to choose one using the keyboard?
I'm happy to continue working on this...
Thanks.
Updated•6 years ago
|
Updated•6 years ago
|
One approach for dealing with column breakpoints would be to select the closest to the cursor position (when conditional panel is opened).
(possibly with getBreakpointsAtLine
from src/selectors/breakpointAtLocation.js
.)
The cursor position is rendered to the DOM inside the Editor/SourceFooter, src/components/Editor/Footer.js
, and I'm contemplating how to pass cursorPosition and line breakpoints to the Props in src/components/Editor/ConditionalPanel.js
.
Does this seem like a reasonable approach for handling opening conditional panel with the keyboard shortcut?
Is it reasonable to create a new action
that would get the cursor position?
Comment 4•6 years ago
|
||
I think using the cursor position would be a good idea! I don't think we need a new action but a function in the utils/editor
directory. We'll probably find some edge cases along the way. Worst case scenario is we default to the first column position and use that for the conditional add/edit.
Previously, the conditional panel did not correctly load breakpoint conditions when opened with the keyboard shortcut.
This patch loads the conditions, and selects the closest active column breakpoint on same line as cursor position in the Editor.
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Comment 7•5 years ago
|
||
Backed out by: https://hg.mozilla.org/integration/autoland/rev/489507a3818665e2bc0992fbe90834d4fc320e2c
Updated•5 years ago
|
Comment 10•5 years ago
|
||
Backed out changeset 1630eaa0a61c (bug 1543261) for causing devtools failure on browser_dbg-breakpoints-cond-source-maps.js CLOSED TREE
Backout revision https://hg.mozilla.org/integration/autoland/rev/b2f41d635be8f3a5be35ae47316b7352d18b7465
Failure log https://treeherder.mozilla.org/logviewer.html#?job_id=267284989&repo=autoland
Alex can you please take a look?
Comment 11•5 years ago
|
||
Comment 12•5 years ago
|
||
bugherder |
Updated•5 years ago
|
Description
•