[webkit-reviews] review granted: [Bug 215982] Remove comparePositions and make VisiblePosition improvements : [Attachment 407705] Patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has granted Darin Adler <darin at apple.com>'s request
for review:
Bug 215982: Remove comparePositions and make VisiblePosition improvements
https://bugs.webkit.org/show_bug.cgi?id=215982

Attachment 407705: Patch

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




--- Comment #9 from Sam Weinig <sam at webkit.org> ---
Comment on attachment 407705
  --> https://bugs.webkit.org/attachment.cgi?id=407705
Patch

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

> Source/WebCore/editing/TextAffinity.h:31
> +enum Affinity : bool { Upstream, Downstream };

I think these would read clearer at callsites as an enum class, but I could see
arguments either way and this is a clear improvement.

> Source/WebCore/editing/VisibleUnits.cpp:1065
> +    if (auto box = visiblePosition.inlineBoxAndOffset().box) {

A thought for the future. Given how often callers of inlineBoxAndOffset() want
just the box, is there a worthwhile optimization of having a version that
doesn't return the offset? Can any computation be avoided?