[webkit-reviews] review granted: [Bug 213564] [iOS] -_requestTextInputContextsInRect cannot find empty Quip spreadsheet title : [Attachment 402674] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Daniel Bates
<dbates at webkit.org>'s request for review:
Bug 213564: [iOS] -_requestTextInputContextsInRect cannot find empty Quip
spreadsheet title
https://bugs.webkit.org/show_bug.cgi?id=213564

Attachment 402674: Patch

https://bugs.webkit.org/attachment.cgi?id=402674&action=review




--- Comment #5 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 402674
  --> https://bugs.webkit.org/attachment.cgi?id=402674
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=402674&action=review

> Source/WebCore/ChangeLog:17
> +	   solution without forcing more line box creation. It's clear to me
that the customer
> +	   experience will be significantly better if the focused editable no
line box case
> +	   is handled, but it's unclear whether that can be topped by patching
everything to
> +	   track all editable inlines without a line box because they are not
discoverable.
> +	   (They can only be found when focused using a keyboard or random
tapping/clicking).

Maybe you don't need all these words.

> Source/WebCore/platform/graphics/FloatRect.cpp:57
> +bool FloatRect::intersectsEvenIfEmpty(const FloatRect& other) const

This is ambiguous about which rect might be empty. Is it |this| or other or
both?