Bug 13291 - REGRESSION (r19595): WebViewDidBeginEditingNotification not posted when focusing with the mouse
Summary: REGRESSION (r19595): WebViewDidBeginEditingNotification not posted when focus...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: 523.x (Safari 3)
Hardware: Macintosh OS X 10.4
: P1 Major
Assignee: Nobody
URL:
Keywords: Regression
Depends on:
Blocks:
 
Reported: 2007-04-05 14:30 PDT by mitz
Modified: 2007-04-19 08:07 PDT (History)
1 user (show)

See Also:


Attachments
Move the call to didBeginEditing() back to Document::setFocusedNode() (487.76 KB, patch)
2007-04-06 00:24 PDT, mitz
hyatt: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mitz 2007-04-05 14:30:55 PDT
The WebViewDidBeginEditingNotification isn't sent when editing begins as the result of clicking and thus focusing an editable element. It is posted when tabbing into the element.

Regressed in <http://trac.webkit.org/projects/webkit/changeset/19595>. updateFocusAppearance() is only called in (possible delayed) response to focus().
Comment 1 mitz 2007-04-05 23:51:19 PDT
The regression was detected by a few tests! <http://trac.webkit.org/projects/webkit/changeset/19605>
Comment 2 mitz 2007-04-06 00:24:53 PDT
Created attachment 13976 [details]
Move the call to didBeginEditing() back to Document::setFocusedNode()
Comment 3 Dave Hyatt 2007-04-06 00:45:57 PDT
Comment on attachment 13976 [details]
Move the call to didBeginEditing() back to Document::setFocusedNode()

I don't think the fix is that simple.  The problem I was originally fixing involved the issue of being focused at a time when you don't yet have a renderer.

I'm pretty sure in that case didBeginEditing is being called too early.  My movement into updateFocusAppearance made didBeginEditing fire in places where it was not before.  

(This is also why I expected results to change in the tests and I guess got confused into thinking failures were changes for the better.)
Comment 4 Dave Hyatt 2007-04-06 00:51:12 PDT
Comment on attachment 13976 [details]
Move the call to didBeginEditing() back to Document::setFocusedNode()

Actually going to +.  I still think it's the wrong behavior to fire this notification before the renderer is constructed, but it wasn't relevant to the bug I was fixing and is no worse than it was before.

We should have a followup bug to track this though.
Comment 5 mitz 2007-04-06 16:17:42 PDT
Comment on attachment 13976 [details]
Move the call to didBeginEditing() back to Document::setFocusedNode()

The two tests updated in <http://trac.webkit.org/projects/webkit/changeset/20762> will probably be affected by this patch and need updated results checked in.
Comment 6 Sam Weinig 2007-04-19 08:07:43 PDT
Landed in r20950.