[webkit-reviews] review denied: [Bug 5230] WebHTMLView (WebNSTextInputSupport) - "characterIndexForPoint: not yet implemented" : [Attachment 4140] proposed implementation of characterIndexForPoint

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Sun Oct 2 14:04:27 PDT 2005


Eric Seidel <macdome at opendarwin.org> has denied Evan Gross
<evan at rainmakerinc.com>'s request for review:
Bug 5230: WebHTMLView (WebNSTextInputSupport) - "characterIndexForPoint: not
yet implemented"
http://bugzilla.opendarwin.org/show_bug.cgi?id=5230

Attachment 4140: proposed implementation of characterIndexForPoint
http://bugzilla.opendarwin.org/attachment.cgi?id=4140&action=edit

------- Additional Comments from Eric Seidel <macdome at opendarwin.org>
A couple comments:

1.  I assume you have some sort of test for this?  It's not immediately obvious
to me that [self convertPoint:[[self window] convertScreenToBase:thePoint]
fromView:nil]; is the right way to get the point in the proper coordinates.  (I
assume the point passed to you is in screen coords? not window or view coords?)


2.  You have several lines with tabs instead of spaces.  According to our
coding style guidelines we use tabs, and not spaces:
http://webkit.opendarwin.org/blog/?page_id=25

3.  The last else is not needed (and has been known to confuse	stupider
versions of gcc into thinking you can reach the end of the function w/o
returning.)

Other than that the patch looks fine.  Someone who knows more about the Obj-C
DOM should review this however.

Thanks for the patch!



More information about the webkit-reviews mailing list