[webkit-reviews] review denied: [Bug 45071] Only intercept ESC key press when autocorrection UI is visible. : [Attachment 67441] proposed patch (v2)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Sep 13 11:38:05 PDT 2010


mitz at webkit.org has denied jpu at apple.com's request for review:
Bug 45071: Only intercept ESC key press when autocorrection UI is visible.
https://bugs.webkit.org/show_bug.cgi?id=45071

Attachment 67441: proposed patch (v2)
https://bugs.webkit.org/attachment.cgi?id=67441&action=review

------- Additional Comments from mitz at webkit.org
View in context:
https://bugs.webkit.org/attachment.cgi?id=67441&action=prettypatch

> WebCore/editing/EditorCommand.cpp:1130
> +static bool supportedDismissCorrectionPanel(Frame* frame,
EditorCommandSource source)
> +{
> +    if (supportedFromMenuOrKeyBinding(frame, source))
> +	   return frame->editor()->isShowingCorrectionPanel();
> +    return false;
> +}
Personally, I would write this as
    return supportedFromMenuOrKeyBinding(frame, source) &&
frame->editor()->isShowingCorrectionPanel();

> WebKit/mac/WebCoreSupport/WebEditorClient.mm:842
> +bool WebEditorClient::isShowingCorrectionPanel() {
> +    return m_correctionPanelTag != InvalidCorrectionPanelTag;
> +}
The opening brace should go on its own line.

r- so you can fix that style issue and consider the other comment.


More information about the webkit-reviews mailing list