[webkit-reviews] review denied: [Bug 96508] [chromium] WebViewImpl::selectionBounds should return start/end on Android : [Attachment 163621] husky at chromium.org

[bugzilla-daemon at webkit.org]

Ryosuke Niwa (vacation: 8/26-9/10) <rniwa at webkit.org> has denied Iain Merrick
<husky at chromium.org>'s request for review:
Bug 96508: [chromium] WebViewImpl::selectionBounds should return start/end on
Android
https://bugs.webkit.org/show_bug.cgi?id=96508

Attachment 163621: husky at chromium.org
https://bugs.webkit.org/attachment.cgi?id=163621&action=review

------- Additional Comments from Ryosuke Niwa (vacation: 8/26-9/10)
<rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=163621&action=review


> Source/WebKit/chromium/src/WebViewImpl.cpp:2266
> +// "Start" always precedes "end". "Base" is where the user's selection
gesture
> +// started, "end" is where it finished. Most platforms want base/extent
here.
> +// Android wants start/end even if base/extent are reversed.
> +// FIXME: This shouldn't be OS-specific. http://webkit.org/b/96508

This is very confusing. Why are arguments called start and end, and yet they
expect it to be base and extent? Furthermore, they aren't base and extent.
They're anchor and focus.
The terminology here is all messed up. Please fix them.

Also, we should probably add a new enum like ShouldUseAnchorAndFocus with
values UseAnchorAndFocus and UseStartAndEnd instead of hard-coding if-endif
like this.