Bug 64824

Summary: [Mac] Need to make sure autocorrection panel is dismissed in Document::setFocusNode().
Product: WebKit Reporter: Jia Pu <jiapu.mail>
Component: HTML EditingAssignee: Jia Pu <jiapu.mail>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, ddavidso, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac (Intel)   
OS: OS X 10.6   
Attachments:
Description Flags
Patch v1
none
Patch v2
none
Patch v3 none

Jia Pu
Reported 2011-07-19 14:09:26 PDT
On search webpages, when moving focus away from text field, autocorrection panel won't get dismissed. <rdar://problem/9624232>
Attachments
Patch v1 (1.36 KB, patch)
2011-07-19 14:20 PDT, Jia Pu
no flags
Patch v2 (1.23 KB, patch)
2011-07-20 14:58 PDT, Jia Pu
no flags
Patch v3 (1.27 KB, patch)
2011-07-21 12:14 PDT, Jia Pu
no flags
Jia Pu
Comment 1 2011-07-19 14:20:06 PDT
Created attachment 101386 [details] Patch v1
Darin Adler
Comment 2 2011-07-19 14:27:05 PDT
Comment on attachment 101386 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=101386&action=review > Source/WebCore/dom/Document.cpp:3098 > + frame()->editor()->dismissCorrectionPanelAsIgnored(); It seems strange and wrong to put this inside the check for oldFocusedNode->inDetach. It’s irritating that we have to actually utter the words “correction panel” here, but I can’t see a better way to structure this.
Jia Pu
Comment 3 2011-07-19 14:37:43 PDT
(In reply to comment #2) > (From update of attachment 101386 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=101386&action=review > > > Source/WebCore/dom/Document.cpp:3098 > > + frame()->editor()->dismissCorrectionPanelAsIgnored(); > > It seems strange and wrong to put this inside the check for oldFocusedNode->inDetach. Darin, could you elaborate a bit. I don't know these code well enough to clearly know where to put this call. > > It’s irritating that we have to actually utter the words “correction panel” here, but I can’t see a better way to structure this. How about calling dismissal in Editor::didEndEditing(), which is called further down in the function? But that's still inside the if statement you mentioned earlier.
Darin Adler
Comment 4 2011-07-20 12:12:12 PDT
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 101386 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=101386&action=review > > > > > Source/WebCore/dom/Document.cpp:3098 > > > + frame()->editor()->dismissCorrectionPanelAsIgnored(); > > > > It seems strange and wrong to put this inside the check for oldFocusedNode->inDetach. > > Darin, could you elaborate a bit. I don't know these code well enough to clearly know where to put this call. I believe that the call to dismiss the correction panel should be made even if the old node is in the middle of the detach process. The rest of what’s inside the if statement body is about that old node directly, and so it should not be done if the node is in the middle of the detach process. But dismissing the correction panel seems to only obliquely be about the node itself, and so seems that it should be done even if the node happens to be in the middle of the detach process. > > It’s irritating that we have to actually utter the words “correction panel” here, but I can’t see a better way to structure this. > > How about calling dismissal in Editor::didEndEditing(), which is called further down in the function? But that's still inside the if statement you mentioned earlier. I think that’s a good idea. As long as is didEndEditing called often enough. Maybe I’m wrong about the inDetach thing.
Jia Pu
Comment 5 2011-07-20 14:58:13 PDT
Created attachment 101516 [details] Patch v2
WebKit Review Bot
Comment 6 2011-07-21 12:01:32 PDT
Comment on attachment 101516 [details] Patch v2 Rejecting attachment 101516 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-02', '--port..." exit_code: 1 Last 500 characters of output: 085e4efceb0bca35c887ff21de2e522a6394c8ea r91483 = 4f1989c27f3f49d3fb006fcdfadc2a139b2f5f91 Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/origin/master. Updating chromium port dependencies using gclient... ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Full output: http://queues.webkit.org/results/9193770
Jia Pu
Comment 7 2011-07-21 12:14:32 PDT
Created attachment 101625 [details] Patch v3 Added in "Reviewed by ..." line to make build bot happy.
WebKit Review Bot
Comment 8 2011-07-22 00:17:01 PDT
Comment on attachment 101625 [details] Patch v3 Clearing flags on attachment: 101625 Committed r91559: <http://trac.webkit.org/changeset/91559>
WebKit Review Bot
Comment 9 2011-07-22 00:17:06 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.