Bug 46679

Summary: Delete and forward delete shouldn't start autocorrection panel timer.
Product: WebKit Reporter: Jia Pu <jiapu.mail>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, adele, commit-queue, eric, jiapu.mail, webkit-ews
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.6   
Attachments:
Description Flags
Proposed patch (v1)
adele: review-, commit-queue: commit-queue-
Proposed patch (v2)
mitz: review-
Proposed patch (v3) none

Jia Pu
Reported 2010-09-27 17:13:02 PDT
<rdar://problem/8463917> Autocorrection panel should not show up when user is deleting or forward deleting.
Attachments
Proposed patch (v1) (3.30 KB, patch)
2010-09-27 18:00 PDT, Jia Pu
adele: review-
commit-queue: commit-queue-
Proposed patch (v2) (3.56 KB, patch)
2010-09-28 14:03 PDT, Jia Pu
mitz: review-
Proposed patch (v3) (3.50 KB, patch)
2010-09-28 16:22 PDT, Jia Pu
no flags
Jia Pu
Comment 1 2010-09-27 18:00:16 PDT
Created attachment 69004 [details] Proposed patch (v1)
WebKit Commit Bot
Comment 2 2010-09-28 10:49:35 PDT
Comment on attachment 69004 [details] Proposed patch (v1) Rejecting patch 69004 from commit-queue. Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'build', '--no-clean', '--no-update', '--build', '--build-style=both', '--quiet']" exit_code: 2 Building WebKit Failed to run "['WebKitTools/Scripts/build-webkit', '--debug']" exit_code: 1 Full output: http://queues.webkit.org/results/4059172
Adele Peterson
Comment 3 2010-09-28 11:42:35 PDT
Comment on attachment 69004 [details] Proposed patch (v1) I'm not sure why the cq failed- trying again...
Eric Seidel (no email)
Comment 4 2010-09-28 11:53:06 PDT
I'm not sure either. I suspect this is a bug from the CommitQueueTask rewrite. I suspect we're not raising the right ScriptError, thus we're losing the log information?
WebKit Commit Bot
Comment 5 2010-09-28 12:01:11 PDT
Comment on attachment 69004 [details] Proposed patch (v1) Rejecting patch 69004 from commit-queue. Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'build', '--no-clean', '--no-update', '--build', '--build-style=both', '--quiet']" exit_code: 2 Building WebKit Failed to run "['WebKitTools/Scripts/build-webkit', '--debug']" exit_code: 1 Full output: http://queues.webkit.org/results/4111094
Eric Seidel (no email)
Comment 6 2010-09-28 12:06:21 PDT
This patch doesn't seem to build correctly on the mac. However the commit-queue's rejection path is not correctly spitting out the build log. Investigating.
Eric Seidel (no email)
Comment 7 2010-09-28 12:09:02 PDT
http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask.py#L142 is the path it should be hitting. It's possible somehow the ScriptError which was saved is losing its log?
Adele Peterson
Comment 8 2010-09-28 12:10:53 PDT
I'm building this locally, and will land manually eventually.
Adam Barth
Comment 9 2010-09-28 12:43:37 PDT
> I suspect we're not raising the right ScriptError, thus we're losing the log information? The problem is that the bug comment is now coming from the master process, but the child process never outputs any build spew. We probably want the child to print some of the build spew on failure so the parent can see it.
Adele Peterson
Comment 10 2010-09-28 12:48:30 PDT
TypingCommand.cpp:286: warning: unused parameter 'commandType' This is the warning that I'm getting.
Adele Peterson
Comment 11 2010-09-28 12:48:52 PDT
Comment on attachment 69004 [details] Proposed patch (v1) r- so Jia can fix the warning.
Jia Pu
Comment 12 2010-09-28 14:03:00 PDT
Created attachment 69099 [details] Proposed patch (v2) Fixed a build failure on non-Mac OSX platforms. Not sure if this is cleanest way to fix it. Any suggestion from reviewers is welcome.
Adele Peterson
Comment 13 2010-09-28 14:07:52 PDT
Comment on attachment 69099 [details] Proposed patch (v2) I'm not sure about this. I was seeing the warning on 10.6.4.
mitz
Comment 14 2010-09-28 14:17:37 PDT
Comment on attachment 69099 [details] Proposed patch (v2) View in context: https://bugs.webkit.org/attachment.cgi?id=69099&action=review > WebCore/editing/TypingCommand.cpp:321 > +#if PLATFORM(MAC) && !defined(BUILDING_ON_TIGER) && !defined(BUILDING_ON_LEOPARD) > void TypingCommand::typingAddedToOpenCommand(ETypingCommand commandTypeForAddedTyping) > +#else > +void TypingCommand::typingAddedToOpenCommand(ETypingCommand) > +#endif The complaint was about markMisspellingsAfterTyping() having an unused (in some configurations) parameter, not about this method, so I’m not sure why you’re changing this one. I think it will break all builds. The way to fix the warning about markMisspellingsAfterTyping() is to add UNUSED_PARAM(commandType) for the configurations that don’t use commandType. You’ll need to make sure that you’re including <wtf/UnusedParam.h>—but I think you have it by way of including Editor.h.
Early Warning System Bot
Comment 15 2010-09-28 14:24:44 PDT
Jia Pu
Comment 16 2010-09-28 14:36:33 PDT
(In reply to comment #14) > (From update of attachment 69099 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=69099&action=review > > > WebCore/editing/TypingCommand.cpp:321 > > +#if PLATFORM(MAC) && !defined(BUILDING_ON_TIGER) && !defined(BUILDING_ON_LEOPARD) > > void TypingCommand::typingAddedToOpenCommand(ETypingCommand commandTypeForAddedTyping) > > +#else > > +void TypingCommand::typingAddedToOpenCommand(ETypingCommand) > > +#endif > > The complaint was about markMisspellingsAfterTyping() having an unused (in some configurations) parameter, not about this method, so I’m not sure why you’re changing this one. I think it will break all builds. > Sorry, totally messed up this one. Let me generate another patch, and test building it on both platforms
Jia Pu
Comment 17 2010-09-28 16:22:23 PDT
Created attachment 69127 [details] Proposed patch (v3)
WebKit Commit Bot
Comment 18 2010-09-28 17:14:26 PDT
Comment on attachment 69127 [details] Proposed patch (v3) Clearing flags on attachment: 69127 Committed r68613: <http://trac.webkit.org/changeset/68613>
WebKit Commit Bot
Comment 19 2010-09-28 17:14:33 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.