Bug 42939 - WebEditorClient::didBeginEditing is never called in WebKit2
Summary: WebEditorClient::didBeginEditing is never called in WebKit2
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh Intel OS X 10.6
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 43379
  Show dependency treegraph
 
Reported: 2010-07-24 18:00 PDT by Alexey Proskuryakov
Modified: 2010-08-15 18:59 PDT (History)
3 users (show)

See Also:


Attachments
proposed patch (45.51 KB, patch)
2010-08-03 13:25 PDT, Alexey Proskuryakov
sam: review+
Details | Formatted Diff | Diff
Patch (4.27 KB, patch)
2010-08-13 19:03 PDT, Jon Honeycutt
sam: review-
Details | Formatted Diff | Diff
Patch v2 (3.93 KB, patch)
2010-08-15 16:36 PDT, Jon Honeycutt
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2010-07-24 18:00:41 PDT
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.
Comment 1 Alexey Proskuryakov 2010-08-03 03:43:24 PDT
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.
Comment 2 Alexey Proskuryakov 2010-08-03 13:25:08 PDT
Created attachment 63373 [details]
proposed patch

This fixes about 500 tests.
Comment 3 Sam Weinig 2010-08-03 13:26:12 PDT
(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.
Comment 4 Sam Weinig 2010-08-03 13:27:14 PDT
Comment on attachment 63373 [details]
proposed patch

Heh, or that. r=me
Comment 5 Alexey Proskuryakov 2010-08-03 23:54:14 PDT
Committed <http://trac.webkit.org/changeset/64631>.
Comment 6 Jon Honeycutt 2010-08-13 15:17:29 PDT
This issue still exists for WebKit2 on Windows; reopening.
Comment 7 Jon Honeycutt 2010-08-13 17:44:52 PDT
This may be because WebKitTestRunner is not focusing the view window.
Comment 8 Jon Honeycutt 2010-08-13 19:03:39 PDT
Created attachment 64399 [details]
Patch
Comment 9 Sam Weinig 2010-08-14 14:23:26 PDT
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.
Comment 10 Jon Honeycutt 2010-08-15 16:36:58 PDT
Created attachment 64460 [details]
Patch v2
Comment 11 Jon Honeycutt 2010-08-15 18:59:16 PDT
Landed in r65390. Thanks, Sam!