1580591 - Watchpoint UI follow up items

Status: RESOLVED DUPLICATE of bug 1582298

(DevTools :: Debugger, enhancement, P3)

Description

[Jason Laster [:jlast]]

Lists a bunch of UI/UX polish items

UI Polish:

1. Add a new "Break on..." submenu inspired by dom bps
2. The watchpoint icon could be aligned better (maybe 1px lower would be enough):
3. Hover style on the watchpoint icon is almost invisible in dark mode:
4. There's no way to tell if a watchpoint is a get or a set (or both)
5. When a watchpoint is set on a node, right click doesn't do anything. I think we should show a context menu to be able to add a set/get watchpoint, and new items to remove get / set watchpoint on the variable.

Edge Cases:

1. Adding a watchpoint to a primitive doesn't work (size of a Set).
2. Right clicking on a Set <entries> item throws in the browser console:
3. Adding a watchpoint to an array item collapse the array in the scopes panel and turn the item into a getter that resolves to "Restricted"
4. I see the possibility of a leak here as we currently have no way to remove a watchpoint after the scopes panel is emptied (when resuming).
5. Adding a watchpoint to an object that is in a bucket or a <default properties> fails
6. Hovering on an object, does not format the property

Architecture:

1. consider removing redux duplication between loadedProperties and watchpoints

New Features:

1. When you resume, there's no way to see your watchpoints. I don't know if it is possible to show them and automatically remove them...

Comment 1

[Jason Laster [:jlast]]

Brian, do you know what we should do for adding a watchpoint to a primitive like size in a Set?

Comment 2

[Brian Hackett [Laid off!]]

(In reply to Jason Laster [:jlast] from comment #1)

Brian, do you know what we should do for adding a watchpoint to a primitive like size in a Set?

It seems like it would be best if certain properties can't have watchpoints set on them, if we know that they won't behave as expected. That includes Set.size, Array.length, properties of proxies, etc. For Set.size you could get most of the way there by modifying the prototype's add(), clear(), etc. methods to include a trap, but that wouldn't work if the page had already copied those methods somewhere else before the point at which the watchpoint was added. Another option would be a C++ trap for reading/writing the internal set size, but it's a non-trivial change to add such a thing and would get a lot more complicated with Array.length in particular.

Comment 3

[Jason Laster [:jlast]]

Attachment: bug.png

Comment 4

[Jason Laster [:jlast]]

^ pausing can throw errors

it is also possible to get a blank screen

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Further possible followups:

1. How to see list of all existing watchpoints? And perhaps switch them between set/get?
2. How to remove/disable all watchpoints?
3. Can we break on HTML Nodes props? This could be real powerful feature for the web-compat team.

Honza

Comment 6

[Miriam]

The pause reason for set watchpoints is not correct. Easy bug to fix.

https://bugzilla.mozilla.org/show_bug.cgi?id=1582182

Comment 7

[Miriam]

I think we should disable watchpoints if we disable breakpoints. This isn't exactly a UI bug but I will put it here for now:

https://bugzilla.mozilla.org/show_bug.cgi?id=1582193

Comment 8

[Jason Laster [:jlast]]

closing as everything has been converted to bugs