[webkit-reviews] review denied: [Bug 103520] [EFL][GTK] context-menu-suggestions.html fails : [Attachment 178211] proposed patch - right click makes selection on misspelled word

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Dec 9 23:57:35 PST 2012


Hajime Morrita <morrita at google.com> has denied Grzegorz Czajkowski
<g.czajkowski at samsung.com>'s request for review:
Bug 103520: [EFL][GTK] context-menu-suggestions.html fails
https://bugs.webkit.org/show_bug.cgi?id=103520

Attachment 178211: proposed patch - right click makes selection on misspelled
word
https://bugs.webkit.org/attachment.cgi?id=178211&action=review

------- Additional Comments from Hajime Morrita <morrita at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=178211&action=review


The basic approach looks good. I'm asking for some house keeping.

> Source/WebCore/editing/Editor.cpp:4
> + * Copyright (C) 2012 Samsung Electronics

I would hold back doing this unless the size of contribution is significant.

> Source/WebCore/editing/Editor.cpp:1691
> +bool Editor::isWordMisspelledAtCaret(Node* clickedNode) const

If the code is based on Chromium's implementation, Please file a bug and add
FIXME on Chromium side to use this code there.

> Source/WebCore/page/EventHandler.cpp:2882
> +    bool selectOnCtxMenu =
m_frame->editor()->behavior().shouldSelectOnContextualMenuClick()

Please don't use abbreviation like ctx.

> Source/WebCore/page/EventHandler.cpp:2886
> +    bool selectOnCtxMenuForMisspelledWord =
m_frame->editor()->behavior().shouldSelectOnContextualMenuClickForMisspelledWor
d()

Ditto.


More information about the webkit-reviews mailing list