WebEditorClient::didBeginEditing is never called in WebKit2

• Component: WebKit2
• Status: RESOLVED FIXED

[Alexey Proskuryakov]

E.g. for editing/deleting/4845371.html, all expected delegates but this one are logged.

I suspect that this is caused by some focus handling issues, as client()->didBeginEditing is called from Document::setFocusedNode(), and that one is never invoked on this test.

[Alexey Proskuryakov]

This is caused by an incomplete implementation of becomeFirstResponder. WebKit1 version does much more work - specifically, this problem is caused by a missing FocusController::setFocusedFrame() call.

I'm not yet sure what the design here is. It seems strange for WebCore focus handling logic to be partially in WebKit.

[Alexey Proskuryakov]

Created attachment 63373 [details]
proposed patch

This fixes about 500 tests.

[Sam Weinig]

(In reply to comment #1)
> This is caused by an incomplete implementation of becomeFirstResponder. WebKit1 version does much more work - specifically, this problem is caused by a missing FocusController::setFocusedFrame() call.

From the looking at the windows code, it looks like something like

void WebPage::setFocused(bool isFocused)
{
    FocusController* focusController = m_page->focusController();
    focusController->setFocused(isFocused);

    if (isFocused) {
        if (!focusController->focusedFrame())
            focusController->setFocusedFrame(m_page->mainFrame());
    }
}

should do most of the necessary work.

[Sam Weinig]

Comment on attachment 63373 [details]
proposed patch

Heh, or that. r=me

[Alexey Proskuryakov]

Committed <http://trac.webkit.org/changeset/64631>.

[Jon Honeycutt]

This issue still exists for WebKit2 on Windows; reopening.

[Jon Honeycutt]

This may be because WebKitTestRunner is not focusing the view window.

[Jon Honeycutt]

Created attachment 64399 [details]
Patch

[Sam Weinig]

Comment on attachment 64399 [details]
Patch

I think it would make more sense to put the focus function on the PlatformWebView abstraction. But otherwise this is fine. r- for that change.

[Jon Honeycutt]

Created attachment 64460 [details]
Patch v2

[Jon Honeycutt]

Landed in r65390. Thanks, Sam!