Bug 46679 - Delete and forward delete shouldn't start autocorrection panel timer.
Summary: Delete and forward delete shouldn't start autocorrection panel timer.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.6
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2010-09-27 17:13 PDT by Jia Pu
Modified: 2010-09-28 17:14 PDT (History)
6 users (show)

See Also:


Attachments
Proposed patch (v1) (3.30 KB, patch)
2010-09-27 18:00 PDT, Jia Pu
adele: review-
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Proposed patch (v2) (3.56 KB, patch)
2010-09-28 14:03 PDT, Jia Pu
mitz: review-
Details | Formatted Diff | Diff
Proposed patch (v3) (3.50 KB, patch)
2010-09-28 16:22 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 2010-09-27 17:13:02 PDT
<rdar://problem/8463917>

Autocorrection panel should not show up when user is deleting or forward deleting.
Comment 1 Jia Pu 2010-09-27 18:00:16 PDT
Created attachment 69004 [details]
Proposed patch (v1)
Comment 2 WebKit Commit Bot 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
Comment 3 Adele Peterson 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...
Comment 4 Eric Seidel (no email) 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?
Comment 5 WebKit Commit Bot 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
Comment 6 Eric Seidel (no email) 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.
Comment 7 Eric Seidel (no email) 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?
Comment 8 Adele Peterson 2010-09-28 12:10:53 PDT
I'm building this locally, and will land manually eventually.
Comment 9 Adam Barth 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.
Comment 10 Adele Peterson 2010-09-28 12:48:30 PDT
TypingCommand.cpp:286: warning: unused parameter 'commandType'

This is the warning that I'm getting.
Comment 11 Adele Peterson 2010-09-28 12:48:52 PDT
Comment on attachment 69004 [details]
Proposed patch (v1)

r- so Jia can fix the warning.
Comment 12 Jia Pu 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.
Comment 13 Adele Peterson 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.
Comment 14 mitz 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.
Comment 15 Early Warning System Bot 2010-09-28 14:24:44 PDT
Attachment 69099 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4211001
Comment 16 Jia Pu 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
Comment 17 Jia Pu 2010-09-28 16:22:23 PDT
Created attachment 69127 [details]
Proposed patch (v3)
Comment 18 WebKit Commit Bot 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>
Comment 19 WebKit Commit Bot 2010-09-28 17:14:33 PDT
All reviewed patches have been landed.  Closing bug.