[webkit-reviews] review denied: [Bug 107171] shouldMoveCaretToHorizontalBoundaryWhenPastTopOrBottom should return false on Android : [Attachment 184048] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Chris Hopman <cjhopman at chromium.org>'s
request for review:
Bug 107171: shouldMoveCaretToHorizontalBoundaryWhenPastTopOrBottom should
return false on Android
https://bugs.webkit.org/show_bug.cgi?id=107171

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=184048&action=review


This is not

> Source/WebCore/editing/EditingBehavior.h:49
> -    bool shouldMoveCaretToHorizontalBoundaryWhenPastTopOrBottom() const {
return m_type != EditingWindowsBehavior; }
> +    bool shouldMoveCaretToHorizontalBoundaryWhenPastTopOrBottom() const
> +    {
> +#if PLATFORM(CHROMIUM) && defined(ANDROID)
> +	   return false;
> +#else
> +	   return m_type != EditingWindowsBehavior;
> +#endif
> +    }

This is not the way to fix this. It’s really bad to be putting #if statements
in this header. The whole idea here was to define editing behavior types
instead of having a pile of ifdefs. I am extremely unhappy to see
shouldAllowSpellingSuggestionsWithoutSelection special cased for Chromium; too
bad I was not able to stop this then when it first happened. The right way to
do this is to add more editing behavior types, not add #if statements.