Open Bug 1004678 (dbg-dom-bps) Opened 10 years ago Updated 1 year ago

[meta] Debugger DOM Mutation Breakpoints

Categories

(DevTools :: Debugger, enhancement, P3)

enhancement

Tracking

(Not tracked)

People

(Reporter: jhirsch, Unassigned)

References

(Depends on 10 open bugs, Blocks 2 open bugs, )

Details

(Keywords: meta, Whiteboard: [polish-backlog][difficulty=hard][devtools-ux])

User Story

Getting to know what added a specific element, removed it or changed its attributes requires some way to stop the script execution right at the statement where the change happens.

Attachments

(1 file, 1 obsolete file)

It'd be great to be able to break on DOM mutations and jump to the JS that triggered them.

As seen in Firebug[1] and Chrome devtools[2]

[1] https://getfirebug.com/wiki/index.php/HTML_Panel#Break_On_Mutate
[2] https://developers.google.com/chrome-developer-tools/docs/javascript-debugging#breakpoints-mutation-events
OS: Mac OS X → All
Priority: -- → P2
Hardware: x86 → All
Yes, this is much needed.
Panos: don't we already have a bug for this in the debugger component? If not, would you mind suggesting how to approach this bug?
Flags: needinfo?(past)
I looked at the debugger bugs and I don't see one filed for breaking on DOM mutation. This sounds like mostly a bug for the debugger component, as it seems to me that we would need to add functionality similar to the existing break-on-dom-events. My guess is that we should use mutation observers to get notified about mutations and then, if the JS stack is not already available, break on the next JS bytecode. Bug 717749 adds a breakOnNext() method to the debugger client to do exactly that.

If the mutations happen after returning to the event loop (which seems unlikely) we should add platform support for reporting the JS stack at the point of mutation. Both me and Nick have been dabbling in those areas of the code base lately.

In order to present this feature to the user from the inspector UI in addition to the debugger UI, we would have to go a bit further. If we agree that the best UX is for the toolbox to switch to the debugger on DOM mutation, then we need to implement a background initialization of the debugger client and server without opening the debugger iframe. That shouldn't be too hard to do and it would be necessary in other cases as well, like breaking on network events.
Flags: needinfo?(past)
Thanks Panos.

(In reply to Panos Astithas [:past] from comment #2)
> I looked at the debugger bugs and I don't see one filed for breaking on DOM
> mutation. This sounds like mostly a bug for the debugger component, as it
> seems to me that we would need to add functionality similar to the existing
> break-on-dom-events. My guess is that we should use mutation observers to
> get notified about mutations and then, if the JS stack is not already
> available, break on the next JS bytecode. Bug 717749 adds a breakOnNext()
> method to the debugger client to do exactly that.
Cool. Marking this as depending on bug 717749.
The MutationObserver does seem like a good fit, especially knowing that its `observe` method takes a node as argument, which will come in useful for this feature.
I'll move this bug to the debugger component.

> If the mutations happen after returning to the event loop (which seems
> unlikely) we should add platform support for reporting the JS stack at the
> point of mutation. Both me and Nick have been dabbling in those areas of the
> code base lately.
> 
> In order to present this feature to the user from the inspector UI in
> addition to the debugger UI, we would have to go a bit further. If we agree
> that the best UX is for the toolbox to switch to the debugger on DOM
> mutation, then we need to implement a background initialization of the
> debugger client and server without opening the debugger iframe. That
> shouldn't be too hard to do and it would be necessary in other cases as
> well, like breaking on network events.
It sounds to me like we should file a follow-up bug for this to take place once this bug is fixed.

Darrin is also needinfo'd here to give his input on the UX part for this.
Here's what Firebug does: the context menu for a node in the mkarup-view has 3 options: "break on childlist changed", "break on attribute changed", "break on delete": see this screenshot: https://dl.dropboxusercontent.com/u/714210/firebug-break-on-mutation.png
Component: Developer Tools: Inspector → Developer Tools: Debugger
Depends on: 717749
Flags: needinfo?(dhenein)
Summary: Add "break on mutate" support → Add "break on mutate" support in the debugger
Blocks: 1005825
Summary: Add "break on mutate" support in the debugger → Break on DOM mutation
Flags: needinfo?(dhenein)
Severity: normal → enhancement
Depends on: 1141312
Whiteboard: [devedition-40] → [devedition-40][difficulty=hard]
Whiteboard: [devedition-40][difficulty=hard] → [polish-backlog][difficulty=hard]
(In reply to Patrick Brosset [:pbrosset] [:pbro] [on PTO until August 10th] from comment #3)
...
> > In order to present this feature to the user from the inspector UI in
> > addition to the debugger UI, we would have to go a bit further. If we agree
> > that the best UX is for the toolbox to switch to the debugger on DOM
> > mutation, then we need to implement a background initialization of the
> > debugger client and server without opening the debugger iframe. That
> > shouldn't be too hard to do and it would be necessary in other cases as
> > well, like breaking on network events.
> It sounds to me like we should file a follow-up bug for this to take place
> once this bug is fixed.


Many ages have gone by since this was last considered, and a lot of work has been done on the debugger and inspector in the meantime. I'd like an evaluation of where are in terms of supporting work - Bug 717749 has been resolved for over a year. This is a parity bug with Firebug and Chrome, so I'd like to get it done soon.
Flags: needinfo?(pbrosset)
Flags: needinfo?(past)
Whiteboard: [polish-backlog][difficulty=hard] → [polish-backlog][difficulty=hard][devtools-ux]
Unless I'm missing some recent changes, the status hasn't changed much. Nick filed bug 1141312 with an alternative (and probably better) way to implement this in the platform level, but I believe we can go ahead with the plan outlined above in the meantime. 

The UI is not a trivial amount of work either:
- context menu items in the inspector to add breakpoints
- a new "DOM breakpoints" tab in the debugger's right side pane that lists current breakpoints
- checkboxes to enable/disable these breakpoints
- a new context menu in "DOM breakpoints" that contains our standard disable/enable/remove breakpoint actions from the left side pane context menu
Flags: needinfo?(past)
(In reply to Panos Astithas [:past] from comment #9)
> Unless I'm missing some recent changes, the status hasn't changed much. Nick
> filed bug 1141312 with an alternative (and probably better) way to implement
> this in the platform level, but I believe we can go ahead with the plan
> outlined above in the meantime. 
> 
> The UI is not a trivial amount of work either:
> - context menu items in the inspector to add breakpoints
> - a new "DOM breakpoints" tab in the debugger's right side pane that lists
> current breakpoints
> - checkboxes to enable/disable these breakpoints
> - a new context menu in "DOM breakpoints" that contains our standard
> disable/enable/remove breakpoint actions from the left side pane context menu

Awesome, really appreciate the summary.
Flags: needinfo?(pbrosset)
Hey, I'm interested in this bug, going to assign myself.

I think I will have a few questions about Debugger as I don't have experience working on it, so I will ping you Panos if you don't mind.
Assignee: nobody → mdibaiee
So, I took a look around, listening on markupmutation was trivially easy, as well as adding the contextmenu item.

For the debugger part, I think I should create a new class like |EventListeners| in debugger-controller.js [0], which handles DOM Mutations instead, as well as a view like event-listeners-view.js [1], is that right? I just want to make sure I'm taking the right path.

[0] https://dxr.mozilla.org/mozilla-central/source/browser/devtools/debugger/debugger-controller.js#1543
[1] https://dxr.mozilla.org/mozilla-central/source/browser/devtools/debugger/views/event-listeners-view.js#9
Flags: needinfo?(past)
Chrome has 'DOM Breakpoints' in both the Inspector and Debugger sidebar. It makes sense to me that we would present the breakpoints in both views. Something to consider!
(In reply to Mahdi Dibaiee [:mahdi][:mdibaiee] from comment #12)
> For the debugger part, I think I should create a new class like
> |EventListeners| in debugger-controller.js [0], which handles DOM Mutations
> instead, as well as a view like event-listeners-view.js [1], is that right?
> I just want to make sure I'm taking the right path.

This sounds right to me, but James has been refactoring the debugger frontend lately, so let's see if he has any other plans.

(In reply to Gabriel Luong [:gl] from comment #13)
> Chrome has 'DOM Breakpoints' in both the Inspector and Debugger sidebar. It
> makes sense to me that we would present the breakpoints in both views.

I agree, as long as we can share the entire widget. Let's not write and debug that code twice as we've sometimes done in the past.
Flags: needinfo?(past) → needinfo?(jlong)
(In reply to Mahdi Dibaiee [:mahdi][:mdibaiee] from comment #12)
> So, I took a look around, listening on markupmutation was trivially easy, as
> well as adding the contextmenu item.

I'm not 100% clear the scope of this bug.  Assuming this feature is tied to an individual element selected in the inspector this should work.

If not (like if we want to break on *any* mutation from the debugger), it's not going to work correctly for a couple of reasons:
1) The markupmutation event only fires if the Inspector has been opened
2) The Walker fires a mutation event only if it cares about that node (i.e. it's part of the refMap meaning that it's been expanded in the Inspector): https://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/inspector.js#2776
(In reply to Brian Grinstead [:bgrins] from comment #15)
> (In reply to Mahdi Dibaiee [:mahdi][:mdibaiee] from comment #12)
> > So, I took a look around, listening on markupmutation was trivially easy, as
> > well as adding the contextmenu item.
> 
> I'm not 100% clear the scope of this bug.  Assuming this feature is tied to
> an individual element selected in the inspector this should work.

Actually, since the mutation observer callbacks only fire after the end of the current microtask, I think we will not be able to break on the actual line that we care about.

For instance:

  function funcA() {
    node.setAttribute("foo", "bar"); // We want to pause right here...
    1+1;
    // ... but mutation records are taken now and observer is notified, firing an "attributes" record
  }

  funcA();

I'm not sure if we need Bug 1141312, or Mutation Events, or what.  But I'm fairly sure that the MutationObserver will not provide us what we need to implement a 'break on mutation' feature.  Olli, does this sound right?
Flags: needinfo?(bugs)
That sounds right. I think there is a bug open to have a [ChromeOnly] way to mark MutationObserver to deliver records asap it is safe to run scripts. That would be quite similar to Mutation Events, except that DOMNodeRemoved is dispatched before a node is actually removed from its parent.

Adding such mode to MutationObserver should be actually rather easy to implement. Would it be useful for devtools? One thing to remember is that it can be _a_lot_ slower if there are lots of mutations within one microtask.
(Microtasks were initially added to the web platform to be able to have faster way to observe DOM mutations than Mutation events, and IIRC MutationObservers in Gecko are 7x faster than Mutation Events for observing changes to dom tree)
Flags: needinfo?(bugs) → needinfo?(bgrinstead)
(In reply to Olli Pettay [:smaug] from comment #17)
> That sounds right. I think there is a bug open to have a [ChromeOnly] way to
> mark MutationObserver to deliver records asap it is safe to run scripts.
> That would be quite similar to Mutation Events, except that DOMNodeRemoved
> is dispatched before a node is actually removed from its parent.
> 
> Adding such mode to MutationObserver should be actually rather easy to
> implement. Would it be useful for devtools? One thing to remember is that it
> can be _a_lot_ slower if there are lots of mutations within one microtask.
> (Microtasks were initially added to the web platform to be able to have
> faster way to observe DOM mutations than Mutation events, and IIRC
> MutationObservers in Gecko are 7x faster than Mutation Events for observing
> changes to dom tree)

Would there be a way to toggle that behavior on and off for existing observers and reclaim the performance benefit if it was reverted back to normal?  If so, we could turn the immediate mode on if and only if there is a DOM breakpoint set, and then revert back to the normal mode when that was removed.

The MutationObserver would also need to ignore the 'mergeAttributeRecords' (and any other future merging) functionality when the immediate mode was on.  I'm assuming this would automatically happen if we weren't waiting for microtask to finish, but just mentioning this to be explicit.
Flags: needinfo?(bgrinstead)
It could be a toggle yes, similar to the attribute change merging thing.

And by default it would ignore the merging, since it is always safe to run scripts between
attribute value changes.
I tested existing "Break on attribute modification" functionality in Chrome and Firebug.  I did this test on http://bgrins.github.io/devtools-demos/inspector/mutations.html.  I set 'break on attribute change' on the 'h2.changing' element on that page.

It looks like in Chrome it breaks *before* the attribute is modified (i.e. on the breaks on the line calling setAttribute).  Specifically, it's pausing at this line: https://github.com/bgrins/devtools-demos/blob/dcd89f95e22e783e22fc3e147eb9d0d69a94ff9e/inspector/mutations.html#L34.

So I can run this in the split console:

  changingH2.getAttribute("data-count"); // "11"
  current; // 12

Firebug seems to pause *after* the attribute is modified and is somehow pausing inside of an anonymous frame that has no representation in the source.  You can jump up the callstack to the previous line (the one Chrome is pausing at), and here's what I see when running this in the split console:

  changingH2.getAttribute("data-count"); // "12"
  current; // 12

So, it's already executed the line that triggered the change before pausing.

I think Chrome's behavior is more useful and a little less confusing in the UI (since can pause directly on the line that's *going* to change the attribute), but it must not use any mutation observer or events.  It's probably getting a callback before certain DOM functions happen (maybe this would be covered by Bug 1141312).

However, it sounds like getting something similar to what Firebug has may require less platform work and has less open questions.  Any opinions about how to proceed?  Specifically, is it important for the feature that we pause *before* the mutation happens?  If so, I believe that the Mutation Observer path is a non-starter (but Olli, please correct me if I'm wrong).
Flags: needinfo?(past)
Flags: needinfo?(jgriffiths)
My opinion is that breaking before the change is more consistent with setting a normal breakpoint or breaking on exceptions. They stop *before* the statement is executed.

But if that requires more platform work, you should first do Firebug's implementation. Later on you can change the behavior to stop before the statement is executed.

Sebastian
MutationObserver indeed can't help with break-before-DOM-mutation.
I can think of three question users may want answered by DOM breakpoints:

(1) Did a DOM mutation happen?
(2) Where did it happen?
(3) Why did it happen?

Any of the available options will answer (1), so I believe it is better to have some solution now than wait for the prefect solution later. It also sounds like all of our options will help answer (2), but with varying accuracy. Comment 16 indicates a less than ideal answer, but clearly an answer that the user can put to good use. In order to provide an accurate answer to (3), I believe we need to get the stack that resulted in the DOM mutation happening. It's not clear to me whether Olli's suggestion gives us that (perhaps some/most of the time?) or if we must have Nick's approach. But it does sound that the former is at least an order of magnitude easier to implement than the latter, so I'd be fine if we went with that.
Flags: needinfo?(past)
I wonder... does Firebug actually just call mutationObserver.takeRecords() when a break point is triggered. That would bring the same behavior what I was thinking with the special flag.
(In reply to Olli Pettay [:smaug] from comment #24)
> I wonder... does Firebug actually just call mutationObserver.takeRecords()
> when a break point is triggered. That would bring the same behavior what I
> was thinking with the special flag.

According to this it looks like they may be using Mutation Events: https://github.com/firebug/firebug/blob/d478fe988960291cfc45dfb0435930e196ec0eb4/extension/content/firebug/html/htmlModule.js#L167 and https://github.com/firebug/firebug/blob/fa5feac6c7788c67e70ac366ad1739391c64b876/extension/content/firebug/html/htmlPanel.js#L1163.
(In reply to Brian Grinstead [:bgrins] from comment #25)
> (In reply to Olli Pettay [:smaug] from comment #24)
> > I wonder... does Firebug actually just call mutationObserver.takeRecords()
> > when a break point is triggered. That would bring the same behavior what I
> > was thinking with the special flag.
> 
> According to this it looks like they may be using Mutation Events:
> https://github.com/firebug/firebug/blob/
> d478fe988960291cfc45dfb0435930e196ec0eb4/extension/content/firebug/html/
> htmlModule.js#L167 and
> https://github.com/firebug/firebug/blob/
> fa5feac6c7788c67e70ac366ad1739391c64b876/extension/content/firebug/html/
> htmlPanel.js#L1163.

Yes, Firebug uses Mutation Events for DOM breakpoints. There was a report for replacing them by Mutation Observers.[1][2]

Sebastian

[1] https://github.com/firebug/firebug/issues/6300
[2] https://code.google.com/p/fbug/issues/detail?id=6178
(In reply to Panos Astithas [:past] from comment #23)
> I can think of three question users may want answered by DOM breakpoints:
> 
> (1) Did a DOM mutation happen?
> (2) Where did it happen?
> (3) Why did it happen?
> 
> Any of the available options will answer (1), so I believe it is better to
> have some solution now than wait for the prefect solution later. It also
> sounds like all of our options will help answer (2), but with varying
> accuracy. Comment 16 indicates a less than ideal answer, but clearly an
> answer that the user can put to good use. In order to provide an accurate
> answer to (3), I believe we need to get the stack that resulted in the DOM
> mutation happening. It's not clear to me whether Olli's suggestion gives us
> that (perhaps some/most of the time?) or if we must have Nick's approach.
> But it does sound that the former is at least an order of magnitude easier
> to implement than the latter, so I'd be fine if we went with that.

I don't actually know the exact steps we'd have to follow to get the proposed MutationObserver solution to handle 2 and 3.  I'm not sure if we would even be able to capture the exact line that triggered the mutation (for 2). And I'm not sure what frame we would end up pausing on.
(In reply to Panos Astithas [:past] from comment #14)
> (In reply to Mahdi Dibaiee [:mahdi][:mdibaiee] from comment #12)
> > For the debugger part, I think I should create a new class like
> > |EventListeners| in debugger-controller.js [0], which handles DOM Mutations
> > instead, as well as a view like event-listeners-view.js [1], is that right?
> > I just want to make sure I'm taking the right path.
> 
> This sounds right to me, but James has been refactoring the debugger
> frontend lately, so let's see if he has any other plans.

Go ahead and do you mentioned, Mahdi. I don't want to block other's work. The only thing I'd caution against is refactoring any of the breakpoint or source listing code (which is a lot of the frontend), since I have a semi-large patch coming up to heavily clean it up. I'm also introducing a new way to define controllers and views, and I'm very close to landing (maybe in a week), but I don't want to hold you up. I may find problems and have to fix them.

If this hasn't landed before some of my refactoring lands, maybe I'll help you refactor this into the "new" style. Shouldn't be hard at all, won't change too much, and you'll have already done the hard work.

I haven't read all the details in this bug, but this sounds good. We need to improve the UX of the debugger panel in general, it's hard to put new stuff in there. But maybe we can fit it in.
Flags: needinfo?(jlong)
(In reply to James Long (:jlongster) from comment #28)
> Go ahead and do you mentioned, Mahdi. I don't want to block other's work.
> The only thing I'd caution against is refactoring any of the breakpoint or
> source listing code (which is a lot of the frontend), since I have a
> semi-large patch coming up to heavily clean it up. I'm also introducing a
> new way to define controllers and views, and I'm very close to landing
> (maybe in a week), but I don't want to hold you up. I may find problems and
> have to fix them.

(In reply to Mahdi Dibaiee [:mahdi][:mdibaiee] from comment #29)
> Okay!

We still have some unanswered questions before we can actually get the breakpoints working.  Comment 16 outlines why the original approach won't work and the remaining comments are discussing options.  I guess we could start by using the markupmutation event with the knowledge that we'll need to change that before landing.  You could definitely proceed with the frontend parts though (context menu in inspector, debugger listing of the 'breakpoints').
(In reply to Brian Grinstead [:bgrins] from comment #20)
...
> However, it sounds like getting something similar to what Firebug has may
> require less platform work and has less open questions.  Any opinions about
> how to proceed?  Specifically, is it important for the feature that we pause
> *before* the mutation happens?  If so, I believe that the Mutation Observer
> path is a non-starter (but Olli, please correct me if I'm wrong).

Echoing Panos' 3 points, I think the 'why' is really important. I'm fine an experience *something* like Firebug in the short term as long as we don't implement the weird anonymous stackframe behaviour. We should break at a useful point in the user's code.

+1 for taking this on :)
Flags: needinfo?(jgriffiths)
Okay, here is a prototype using "markupmutation" events. You probably don't want to review the code as there are [currently] useless codes here and there as I was experimenting with things.

It's functioning for most part, except I don't know if "markupmutation"s provide a `url` of where the mutation happened so I can show the line on Debugger's editor.

1. We want to have checkboxes in the list so the user can disable/enable breakpoints. The thing is, in case of event listeners, it's populated by event listeners in the page, but here, we are "adding" breakpoints, so how does the user remove a breakpoint? Are we going to provide that option at all?

2. I think we can use a checkboxy-item in inspector's contextmenu, so it's possible to disable the breakpoint directly from contextmenu, without the need to switch over to debugger, what do you think?

3. Currently I'm checking against an array of nodes to avoid adding duplicate breakpoints for a node, what do you think about using a `Set` in place of an array? Would that be a good idea?

Thanks!
Attachment #8654139 - Flags: review?(past)
Sorry, forgot to `hg add` one of the files.
Attachment #8654139 - Attachment is obsolete: true
Attachment #8654139 - Flags: review?(past)
Attachment #8654142 - Flags: review?(past)
Comment on attachment 8654142 [details] [diff] [review]
Patch 1: Prototype using "markupmutation" events

Review of attachment 8654142 [details] [diff] [review]:
-----------------------------------------------------------------

The frontend bits look nice, but I don't believe this backend approach is going to work. I should probably have flagged this up sooner, but "markupmutation" events are not delivered fast enough for our purposes. Any event that gets transmitted across the remote debugging protocol divide is not going to work, unless the page is paused. This is why the pause-on-DOM-events backend implementation doesn't send a similar "domevent" event to the frontend.

What needs to happen is that the client should indicate to the backend the node(s) that need to be observed for mutations and the server would pause once these mutations happen and let the pause event be handled normally. In more concrete terms, dbg-client.jsm looks OK, but _maybeListenToMutations in script.js must look more like _maybeListenToEvents, only it will use MutationObserver instead of the nsIEventListenerService. _maybeListenToEvents also needs to be hooked into onResume and probably _onWindowReady, too.
Attachment #8654142 - Flags: review?(past) → review-
(In reply to Mahdi Dibaiee [:mahdi][:mdibaiee] from comment #32)
> 1. We want to have checkboxes in the list so the user can disable/enable
> breakpoints. The thing is, in case of event listeners, it's populated by
> event listeners in the page, but here, we are "adding" breakpoints, so how
> does the user remove a breakpoint? Are we going to provide that option at
> all?

I mentioned this in the last bullet point in comment 9: we need a new context menu in the "DOM breakpoints" pane that contains our standard disable/enable/remove breakpoint actions from the left side pane context menu.

> 2. I think we can use a checkboxy-item in inspector's contextmenu, so it's
> possible to disable the breakpoint directly from contextmenu, without the
> need to switch over to debugger, what do you think?

I think that this doesn't scale if you have even a handful of DOM breakpoints and you need to track down the nodes in the markup to disable them. How do you even know which nodes are the breakpoints set at?

Also, the main case I believe a developer will want to disable a breakpoint is when it is hit, but the dev realizes that it's not pertinent to the current debugging session. At that point, the debugger tab will be focused anyway (due to the pause) and using the DOM breakpoints pane will be less work.

> 3. Currently I'm checking against an array of nodes to avoid adding
> duplicate breakpoints for a node, what do you think about using a `Set` in
> place of an array? Would that be a good idea?

I haven't seen the exact code you describe here, but we definitely like using sets in devtools source!
Hey, sorry for the delay, been busy working on a few projects.

I updated and took a look at the code again a few days ago, it seems the debugger code is changing, James, are these your refactors? How long until they are stabilized?
Flags: needinfo?(jlong)
@Mahdi I'm not sure what you saw, bug 1177891 landed which is a small part of the refactoring, yes. I have a much bigger patch to refactor breakpoints and sources. If all you're working in is the event listeners files, you should be fine.
Flags: needinfo?(jlong)
Seems inactive, unassigning for now.
Assignee: mdibaiee → nobody
(In reply to Panos Astithas [:past] from comment #35)
> (In reply to Mahdi Dibaiee [:mahdi][:mdibaiee] from comment #32)
> > 1. We want to have checkboxes in the list so the user can disable/enable
> > breakpoints. The thing is, in case of event listeners, it's populated by
> > event listeners in the page, but here, we are "adding" breakpoints, so how
> > does the user remove a breakpoint? Are we going to provide that option at
> > all?
> 
> I mentioned this in the last bullet point in comment 9: we need a new
> context menu in the "DOM breakpoints" pane that contains our standard
> disable/enable/remove breakpoint actions from the left side pane context
> menu.

Having a special side panel only for DOM breakpoints may not be the best solution. In the future there may be other breakpoints like error breakpoints (bug 1165010), XHR breakpoints (bug 1267144), cookie breakpoints, etc.
So, I believe it's better to create a 'Breakpoints' side panel, which holds all kinds of breakpoints (like Firebug does it). The normal script breakpoints should be listed there, too, for consistency.

> > 2. I think we can use a checkboxy-item in inspector's contextmenu, so it's
> > possible to disable the breakpoint directly from contextmenu, without the
> > need to switch over to debugger, what do you think?
> 
> I think that this doesn't scale if you have even a handful of DOM
> breakpoints and you need to track down the nodes in the markup to disable
> them. How do you even know which nodes are the breakpoints set at?

The Chrome DevTools solve this by adding a point besides the element, like the Inspector panel already does it for the element state. A long time ago I filed a similar request to add a breakpoint column to Firebug's HTML panel[1].

> Also, the main case I believe a developer will want to disable a breakpoint
> is when it is hit, but the dev realizes that it's not pertinent to the
> current debugging session. At that point, the debugger tab will be focused
> anyway (due to the pause) and using the DOM breakpoints pane will be less
> work.

I agree that this will probably be the most common case, but hard to say without having any numbers.
Disabling a breakpoint may also happen through the Inspector UI, e.g. via a context menu option and/or by clicking the breakpoint within the breakpoint column (in case one gets added) while holding down a modifier.

Sebastian

[1] https://github.com/firebug/firebug/issues/4933
See Also: → 1165010, 1267144
See Also: → 895893
See Also: → 821610
Product: Firefox → DevTools
I just ran into this issue again. A <div> was added to my HTML output dynamically and I have no idea where it came from.

Ideally this would even work with conditions, say, only stop the script execution when the added element (or its descendants) has the id "x" or the class "y", or the changed attribute name is "z" or its value is "v".
Is conditional breaking covered by this or should I create a new bug for it depending on this one?

Sebastian
User Story: (updated)
Flags: needinfo?(past)
Flags: needinfo?(past) → needinfo?(jlaster)
Taking this, as we got it in the backlog. This is up in the list, we will probably to start investigations in 65.
Flags: needinfo?(jlaster)
(In reply to :Harald Kirschner :digitarald from comment #42)
> Taking this, as we got it in the backlog. This is up in the list, we will
> probably to start investigations in 65.

Sounds good, though isn't a clear answer to my question. From that statement I get: "We don't know yet whether conditional breaking will also be handled in this bug. We need to investigate that first." Please let me know if I misinterpreted that.

Sebastian
Thanks Sebastian for asking to clarify.

Your statement is correct! The idea of conditional DOM breakpoints, where rules could be for example CSS queries, will be into account when we plan out the feature in its phases.
Summary: Break on DOM mutation → [meta] Break on DOM mutation
Priority: P2 → P3
Summary: [meta] Break on DOM mutation → [meta] Debugger DOM mutation
Alias: dbg-dom-mutation-bps
Summary: [meta] Debugger DOM mutation → [meta] Debugger DOM Mutation Breakpoints
Priority: P3 → P2
Alias: dbg-dom-mutation-bps → dbg-dom-bps
Alias: dbg-dom-bps
Priority: P2 → P3
Depends on: 1574540
Alias: dbg-dom-bps
Depends on: 1550030
Depends on: 1550829
Depends on: 1571907
No longer depends on: 1573863
Depends on: 1573694
Depends on: 1575688
Depends on: 1576219
Depends on: 1577911
Depends on: 1580374
Depends on: 1580396
Depends on: 1586454
Depends on: 1614076
No longer blocks: firebug-gaps
Depends on: 1790057
Severity: normal → S3
Depends on: 1802285
You need to log in before you can comment on or make changes to this bug.