[webkit-reviews] review granted: [Bug 132911] sourceFrameOnScreenForShareItem: can be off by a pixel : [Attachment 231456] Patch v1

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Brady Eidson
<beidson at apple.com>'s request for review:
Bug 132911: sourceFrameOnScreenForShareItem: can be off by a pixel
https://bugs.webkit.org/show_bug.cgi?id=132911

Attachment 231456: Patch v1
https://bugs.webkit.org/attachment.cgi?id=231456&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=231456&action=review


> Source/WebKit/mac/WebCoreSupport/WebContextMenuClient.mm:404
> +	   return NSRect();

Isn't there an NSEmptyRect or similar? I'm not sure this is constructing an
empty rect.

> Source/WebKit/mac/WebCoreSupport/WebContextMenuClient.mm:409
> +	   return NSRect();

Ditto.

> Source/WebKit/mac/WebCoreSupport/WebContextMenuClient.mm:414
> +    // FIXME: Ideally we'd like to convert the content rect to screen
coordinates
> +    // without the lossy float -> int conversion.
> +    // Creating a rounded int rect works well in practice, but might still
lead to
> +    // off-by-one-pixel problems in edge cases.

You should file a bugzilla/radar to track this and reference it here.