Bug 64824 - [Mac] Need to make sure autocorrection panel is dismissed in Document::setFocusNode().
Summary: [Mac] Need to make sure autocorrection panel is dismissed in Document::setFoc...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac (Intel) OS X 10.6
: P2 Normal
Assignee: Jia Pu
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2011-07-19 14:09 PDT by Jia Pu
Modified: 2011-07-22 00:17 PDT (History)
3 users (show)

See Also:


Attachments
Patch v1 (1.36 KB, patch)
2011-07-19 14:20 PDT, Jia Pu
no flags Details | Formatted Diff | Diff
Patch v2 (1.23 KB, patch)
2011-07-20 14:58 PDT, Jia Pu
no flags Details | Formatted Diff | Diff
Patch v3 (1.27 KB, patch)
2011-07-21 12:14 PDT, Jia Pu
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jia Pu 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>
Comment 1 Jia Pu 2011-07-19 14:20:06 PDT
Created attachment 101386 [details]
Patch v1
Comment 2 Darin Adler 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.
Comment 3 Jia Pu 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.
Comment 4 Darin Adler 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.
Comment 5 Jia Pu 2011-07-20 14:58:13 PDT
Created attachment 101516 [details]
Patch v2
Comment 6 WebKit Review Bot 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
Comment 7 Jia Pu 2011-07-21 12:14:32 PDT
Created attachment 101625 [details]
Patch v3

Added in "Reviewed by ..." line to make build bot happy.
Comment 8 WebKit Review Bot 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>
Comment 9 WebKit Review Bot 2011-07-22 00:17:06 PDT
All reviewed patches have been landed.  Closing bug.