[webkit-reviews] review denied: [Bug 25526] [Gtk] Additional support is needed for caret browsing : [Attachment 29992] caretrefactor.patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Xan Lopez <xan.lopez at gmail.com>'s
request for review:
Bug 25526: [Gtk] Additional support is needed for caret browsing
https://bugs.webkit.org/show_bug.cgi?id=25526

Attachment 29992: caretrefactor.patch
https://bugs.webkit.org/attachment.cgi?id=29992&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
executeMoveParagraphBackward and executeMoveParagraphForward implementations
seem flipped.

What does Caret Browsing mean?

Seems this should be a static function:
bool caretBrowsing = frame->settings() &&
frame->settings()->caretBrowsingEnabled();

bool caretBrowsingEnabled(frame)

Seems this should be a nicely-named constant somewhere:
static_cast<EditorCommandSource>(0)

Or there should be a different version of enabledInEditableText which doesn't
take a source.

How does one test this?  We need at least a manual test if not a LayoutTest.