[webkit-reviews] review denied: [Bug 82875] Pressing Return at the bottom of an enclosed contenteditable div makes scrolling to jump : [Attachment 137526] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied Mikhail Naganov
<mnaganov at chromium.org>'s request for review:
Bug 82875: Pressing Return at the bottom of an enclosed contenteditable div
makes scrolling to jump
https://bugs.webkit.org/show_bug.cgi?id=82875

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

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=137526&action=review


> LayoutTests/ChangeLog:11
> +	   Reviewed by NOBODY (OOPS!).

This line should appear before the long description.

> Source/WebCore/ChangeLog:11
> +	   Reviewed by NOBODY (OOPS!).

Ditto.

> Source/WebCore/editing/Editor.cpp:948
> +    VisiblePosition caret(m_frame->selection()->selection().visibleStart());


We don't normally prefer operator= over copy constructor (it's just a syntax
sugar anyways). i.e.
VisiblePosition caret = m_frame->selection()->selection().visibleStart().

> Source/WebCore/editing/Editor.cpp:969
> +    VisiblePosition caret(m_frame->selection()->selection().visibleStart());

> +    bool alignToEdge = isEndOfDocument(caret);

This isn't right. We probably need to do the same whenever we're at the end of
the current scrollable area.
Also see https://bugs.webkit.org/show_bug.cgi?id=64143.

r- because this isn't the right condition.