[Webkit-unassigned] [Bug 51897] Escape should clear search field

[bugzilla-daemon at webkit.org]

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





--- Comment #7 from Kentaro Hara <haraken at google.com>  2011-07-14 17:57:36 PST ---
Thank you for comments!

ojan:
> Looking at EventDispatcher::dispatchEvent, we first fire the event to JS and then call defaultEventHandler. haraken, can you confirm that if you call preventDefault from the esc key's keydown event that we don't clear the search input?

You are right. I confirmed that we can prevent WebCore from calling defaultEventHandler() by calling preventDefault() in JS. 


tkent:
> > I think many existing key-binding code should be switched to keypress.
> > eventSender.keyDown() dispatched keydown, keyup, and keypress.

Indeed we may switch those key-binding codes to keypress, but at the present stage, there are several problems to try to handle those key-bindings in the keypress event. 

[1] xxxxxxInputType::handleKeypressEvent() is not called. 

HTMLInputElement.cpp:
void HTMLInputElement::defaultEventHandler(Event* evt) {
    ...;

    /*** part1 ***/
    // Call the base event handler before any of our own event handling for almost all events in text fields.
    // Makes editing keyboard handling take precedence over the keydown and keypress handling in this function.
    bool callBaseClassEarly = isTextField() && (evt->type() == eventNames().keydownEvent || evt->type() == eventNames().keypressEvent);
    if (callBaseClassEarly) {
        HTMLFormControlElementWithState::defaultEventHandler(evt);
        if (evt->defaultHandled())
            return;
    }

    /*** part2 ***/
    // DOMActivate events cause the input to be "activated" - in the case of image and submit inputs, this means
    // actually submitting the form. For reset inputs, the form is reset. These events are sent when the user clicks
    // on the element, or presses enter while it is the active element. JavaScript code wishing to activate the element
    // must dispatch a DOMActivate event - a click event will not do the job.
    if (evt->type() == eventNames().DOMActivateEvent) {
        m_inputType->handleDOMActivateEvent(evt);
        if (evt->defaultHandled())
            return;
    }

    /*** part3 ***/
    // Use key press event here since sending simulated mouse events
    // on key down blocks the proper sending of the key press event.
    if (evt->isKeyboardEvent() && evt->type() == eventNames().keypressEvent) {
        m_inputType->handleKeypressEvent(static_cast<KeyboardEvent*>(evt));
        if (evt->defaultHandled())
            return;
    }
    ...;
}

What is happening is that since HTMLFormControlElementWithState::defaultEventHandler(evt) calls setDefaultHandled() deep inside the call, the method returns at the end of part1 and thus m_inputType->handleKeypressEvent(static_cast<KeyboardEvent*>(evt)) is never called. 

I guess that this problem can be safely resolved just by reordering the above codes to part3 -> part2 -> part1. 


[2] EventSender::keyDown() does not dispatch a keypress event for control characters, including 'escape'. For example, in chromium...

chromium/EventSender.cpp:
void EventSender::keyDown(const CppArgumentList& arguments, CppVariant* result) {
    ...;

    webview()->handleInputEvent(eventDown);  // keydown event

    m_shell->webViewHost()->clearEditCommand();

    if (generateChar) { // only if it is not a control character...
        eventChar.type = WebInputEvent::Char;
        eventChar.keyIdentifier[0] = '\0';
        webview()->handleInputEvent(eventChar);  // keypress event
    }

    webview()->handleInputEvent(eventUp);  // keyup event
}

Also, I confirmed that mac/EventSendingController.mm dispatches the keypress event only for non-control characters. I am not sure for efl, gtk, qt and win, but anyway, we definitely need per-platform changes. 

I think that it is possible to make the fix so that it always dispatches the keypress event, but I am not sure whether this fix is acceptable.

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