Bug 46839 – Autocorrection shouldn't prompt the same correction after user has edited previous correction.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jia Pu]

<rdar://problem/8476963>

[Jia Pu]

Created attachment 69260 [details]
Proposed patch (v1)

Prior to bug 45709. We use "Replacement" marker to indicate the word that has been autocorrected and need to be marked with underline. Whenever user edits an autocorrected word, in order to remove the underline, we remove the "Replacement" marker. However, doing this also prevents us from keeping track of on which word we have already applied autocorrection. So if the user corrects an autocorrection, we would keep prompting the same autocorrection suggestion.

In bug 45709, we introduced a new marker "CorrectionIndicator" to indicate whether we should draw underline. So now we can remove underline by removing "CorrectionIndicator" marker. This allows us to keep "Replacement" marker around, so that we won't suggest autocorrection on word that already bears "Replacement" marker.

[Darin Adler]

Comment on attachment 69260 [details]
Proposed patch (v1)

Change looks fine. We should come up with a way to do regression tests. It’s not safe to have this code in here but not tested in the WebKit regression test suite.

[Jia Pu]

(In reply to comment #2)
> (From update of attachment 69260 [details])
> Change looks fine. We should come up with a way to do regression tests. It’s not safe to have this code in here but not tested in the WebKit regression test suite.

Agree. I will look into writing test for autocorrection. One of the trick parts is that autocorrection result depends on user's past usage. In other word, we adapt to user feedback. So unless we always run the tests on a freshly installed system, there's no guarantee to get the expected correction result back.

[WebKit Commit Bot]

Comment on attachment 69260 [details]
Proposed patch (v1)

Rejecting patch 69260 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'build-and-test', '--no-clean', '--no-update', '--test', '--quiet', '--non-interactive']" exit_code: 2
Last 500 characters of output:
 tests successful.
Files=14, Tests=304,  1 wallclock secs ( 0.63 cusr +  0.13 csys =  0.76 CPU)
Running build-dumprendertree
Compiling Java tests
make: Nothing to be done for `default'.
Running tests from /Users/abarth/git/webkit-queue/LayoutTests
Testing 21408 test cases.
websocket/tests/simple.html -> failed

Exiting early after 1 failures. 21390 tests run.
498.10s total testing time

21389 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
33 test cases (<1%) had stderr output

Full output: http://queues.webkit.org/results/4229026

[Darin Adler]

Comment on attachment 69260 [details]
Proposed patch (v1)

Trying again because the last failure seemed to be due to a flaky websocket test.

[WebKit Commit Bot]

Comment on attachment 69260 [details]
Proposed patch (v1)

Clearing flags on attachment: 69260

Committed r68902: <http://trac.webkit.org/changeset/68902>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.