[Webkit-unassigned] [Bug 42886] Web Inspector: implement DOM breakpoints

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=42886


Pavel Feldman <pfeldman at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #64585|review?                     |review-
               Flag|                            |




--- Comment #8 from Pavel Feldman <pfeldman at chromium.org>  2010-08-17 12:37:05 PST ---
(From update of attachment 64585)
WebCore/bindings/js/ScriptDebugServer.cpp:210
 +      // FIXME(43332): implement this.
You can mention webkit bugzilla here so that we could distinguish this from radar / chromium entries.

WebCore/inspector/InspectorDOMAgent.cpp:742
 +      Node* node = nodeForId(nodeId);
Move this check above? (This should not happen).


WebCore/inspector/front-end/ElementsTreeOutline.js:767
 +              contextMenu.appendItem(WebInspector.UIString("Stop on subtree modifications"),
You should add these to the WebCore/English.lproj/localizedStrings.js

WebCore/inspector/front-end/BreakpointManager.js:93
 +      setDOMBreakpoint: function(nodeId, type)
Nit: I think these should be declared on DOMNode's prototype.

WebCore/inspector/InspectorDOMAgent.cpp:1075
 +      Vector<Node*> stack(1, innerFirstChild(node));
You have 4 places where you traverse the tree. Could it be extracted into a separate method? A part of r- is for this.

WebCore/inspector/InspectorDOMAgent.cpp:204
 +  enum CachedDOMBreakpointType {
I am not sure why you need this separation? r- unless this is explained.

WebCore/inspector/InspectorDOMAgent.cpp:736
 +  void InspectorDOMAgent::setDOMBreakpoint(long nodeId, long type)
So what happens when you add a breakpoint to the node, then add it to its parent and after that remove it from node?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.