[Webkit-unassigned] [Bug 69137] In input field caret is not blinking after context menu hide

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=69137





--- Comment #11 from Ryosuke Niwa <rniwa at webkit.org>  2011-10-17 11:23:25 PST ---
(From update of attachment 111226)
View in context: https://bugs.webkit.org/attachment.cgi?id=111226&action=review

> Source/WebCore/page/ContextMenuController.cpp:98
> +void ContextMenuController::contextMenuHidden()

"hidden" doesn't sound right to me. It's not like context menu is temporarily hidden from the screen. Dismissed or closed will be better.

> Source/WebCore/page/ContextMenuController.cpp:108
> +    // Resume the caret blinking which suspended in context menu show.

This comment repeats the code.

> Source/WebCore/page/ContextMenuController.cpp:109
> +    frame->eventHandler()->suspendCaretBlinking(false);

This function name seems too general to me. I would name to respondToContextMenuDismiss or something like that to avoid tying it to caret blinking.

> Source/WebCore/page/ContextMenuController.cpp:173
> +    Frame* frame = m_hitTestResult.innerNonSharedNode()->document()->frame();
> +    // Suspend the caret blinking to match native application behavior.
> +    frame->eventHandler()->suspendCaretBlinking(true);

This is not right. We've already suspended caret blinking on keydown. We shouldn't be suspending caret blinking here again.

> Source/WebCore/page/EventHandler.cpp:718
> +void EventHandler::suspendCaretBlinking(bool shouldSuspend)

This function must be renamed to reflect the fact it's called when context menu is closed. It also shouldn't take a bool.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.