[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