[webkit-reviews] review granted: [Bug 33618] Web Inspector: Introduce SourceFrame2 with basic breakpoint / execution line rendering capabilities. : [Attachment 46489] [PATCH] Proposed change
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Jan 13 12:50:09 PST 2010
Timothy Hatcher <timothy at hatcher.name> has granted Pavel Feldman
<pfeldman at chromium.org>'s request for review:
Bug 33618: Web Inspector: Introduce SourceFrame2 with basic breakpoint /
execution line rendering capabilities.
https://bugs.webkit.org/show_bug.cgi?id=33618
Attachment 46489: [PATCH] Proposed change
https://bugs.webkit.org/attachment.cgi?id=46489&action=review
------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>
> + selectionRange: function()
> + {
> + this._selection.range();
> + },
Should this return the range? Make it a getter too.
> + // Paint currenlt line for editable mode only.
Typo.
> + if (this._lineNumberDecorator.mouseDown(location.line, e))
It seems odd to me that a decorator handles events. But it does make it easier.
Can this be named handleMouseDown?
> + this._cursorElement.style.display = "none";
Should be addStyleClass("hidden").
r+, but I suspect the selectionRange function has a bug. But it dosen't look to
be called.
More information about the webkit-reviews
mailing list