[webkit-reviews] review denied: [Bug 44837] Web Inspector: persist DOM breakpoints between page reloads : [Attachment 66047] All comments addressed.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Sep 1 02:38:26 PDT 2010


Yury Semikhatsky <yurys at chromium.org> has denied Pavel Podivilov
<podivilov at chromium.org>'s request for review:
Bug 44837: Web Inspector: persist DOM breakpoints between page reloads
https://bugs.webkit.org/show_bug.cgi?id=44837

Attachment 66047: All comments addressed.
https://bugs.webkit.org/attachment.cgi?id=66047&action=review

------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>
View in context:
https://bugs.webkit.org/attachment.cgi?id=66047&action=prettypatch

> WebCore/inspector/front-end/DOMAgent.js:804
> +	   WebInspector.DOMBreakpoint._ids = {};
Why do we need both integer and string type value? We should either use strings
or intergers everywhere, r- for this. Given that the backend uses integers and
we have named constants in the frontend it seems like we may well get rid of
string values.

> WebCore/inspector/front-end/DOMAgent.js:842
> +	  
WebInspector.DOMBreakpoint._contextMenuLabels[WebInspector.DOMBreakpoint.Types.
NodeRemoved] = WebInspector.UIString("Break on Node Removal");	      
Revert this please.


More information about the webkit-reviews mailing list