Bug 112464 – Copy-paste should not spellcheck when continuous spellcheck is turned off

• Component: New Bugs
• Status: RESOLVED FIXED

[Rouslan Solomakhin]

Copy-paste should not spellcheck when continuous spellcheck is turned off

[Rouslan Solomakhin]

Created attachment 193369 [details]
Patch

[Rouslan Solomakhin]

The new layout test editing/spelling/spellcheck-paste-continuous-disabled.html is a copy of editing/spelling/spellcheck-paste-disabled.html, except that it tests turning off the continuous spellcheck setting in the editor, instead of setting spellcheck="off" attribute on the element.

Chromium bug report: https://code.google.com/p/chromium/issues/detail?id=166827

[Build Bot]

Comment on attachment 193369 [details]
Patch

Attachment 193369 [details] did not pass win-ews (win):
Output: http://webkit-commit-queue.appspot.com/results/17213220

[Tony Chang]

Comment on attachment 193369 [details]
Patch

The GTK and Win failures appear to be symbol export lists that need to be updated. See http://trac.webkit.org/wiki/ExportingSymbols .

[Build Bot]

Comment on attachment 193369 [details]
Patch

Attachment 193369 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17118296

[Rouslan Solomakhin]

Created attachment 193390 [details]
Patch

[Rouslan Solomakhin]

Comment on attachment 193390 [details]
Patch

Exposing symbols in symbols.filter and WebCore.exp.in to see how EWS bots react.

[Build Bot]

Comment on attachment 193390 [details]
Patch

Attachment 193390 [details] did not pass win-ews (win):
Output: http://webkit-commit-queue.appspot.com/results/17172198

[Build Bot]

Comment on attachment 193390 [details]
Patch

Attachment 193390 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17145348

[Build Bot]

Comment on attachment 193390 [details]
Patch

Attachment 193390 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17118327

[Rouslan Solomakhin]

Created attachment 193711 [details]
Patch

[Rouslan Solomakhin]

Comment on attachment 193711 [details]
Patch

I've added the exports to every possible .exp, exp.in, and .order file. The build still fails on my Mac, however. The only way to build successfully is to alter WebKitBuild/Release/DerivedSources/WebCore/WebCore.exp by hand. (Even a clean build with no WebKitBuild directory does not update WebCore.exp.) What have I missed?

[Build Bot]

Comment on attachment 193711 [details]
Patch

Attachment 193711 [details] did not pass win-ews (win):
Output: http://webkit-commit-queue.appspot.com/results/17144454

[Ryosuke Niwa]

(In reply to comment #0)
> Copy-paste should not spellcheck when continuous spellcheck is turned off

Why? Having spellcheck automatically run after copy & paste is a pretty important feature to test.

[Ryosuke Niwa]

I don't object to test continuous spellchecking feature but please don't disable spellchecking for copy & paste just so that we can work around the limitation of continuous spellchecks.

[Rouslan Solomakhin]

From what I understand, continuous spellcheck corresponds to "Check spelling as you type" option in web browsers. A user that has this option turned off would not want to see red squiggly lines after copy-pasting some text. Right?

[Ryosuke Niwa]

(In reply to comment #16)
> From what I understand, continuous spellcheck corresponds to "Check spelling as you type" option in web browsers. A user that has this option turned off would not want to see red squiggly lines after copy-pasting some text. Right?

Ah, sorry, I think I misunderstood what you were trying to do.

[Ryosuke Niwa]

Comment on attachment 193711 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=193711&action=review

Please fix the Windows build before you land.

> Source/WebCore/testing/Internals.cpp:1513
> +    if (!contextDocument() || !contextDocument()->frame() || !contextDocument()->frame()->editor())
> +        return;
> +
> +    if (enabled != contextDocument()->frame()->editor()->isContinuousSpellCheckingEnabled())
> +        contextDocument()->frame()->editor()->toggleContinuousSpellChecking();

It's somewhat strange that this isn't a setting but I guess that's outside of the scope of this bug.

[Build Bot]

Comment on attachment 193711 [details]
Patch

Attachment 193711 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17239094

[Build Bot]

Comment on attachment 193711 [details]
Patch

Attachment 193711 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17211468

[Tony Chang]

Comment on attachment 193711 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=193711&action=review

> Source/WebCore/WebCore.exp.in:2120
> -__ZN7WebCore6Editor32isContinuousSpellCheckingEnabledEv
> +__ZN7WebCore6Editor29toggleContinuousSpellCheckingEv

This appears to be inside an #if PLATFORM(IOS) block.  I think you want to move it up before any of the #ifs.

[Rouslan Solomakhin]

(In reply to comment #21)
> (From update of attachment 193711 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=193711&action=review
> 
> > Source/WebCore/WebCore.exp.in:2120
> > -__ZN7WebCore6Editor32isContinuousSpellCheckingEnabledEv
> > +__ZN7WebCore6Editor29toggleContinuousSpellCheckingEv
> 
> This appears to be inside an #if PLATFORM(IOS) block.  I think you want to move it up before any of the #ifs.

You're right, thank you. I will move the lines in the next patch.

[Rouslan Solomakhin]

Created attachment 193876 [details]
Patch for landing

[Rouslan Solomakhin]

Comment on attachment 193876 [details]
Patch for landing

Let's wait for EWS bots to validate the exports.

[Build Bot]

Comment on attachment 193876 [details]
Patch for landing

Attachment 193876 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17189688

New failing tests:
editing/spelling/spellcheck-paste-continuous-disabled.html

[Build Bot]

Comment on attachment 193876 [details]
Patch for landing

Attachment 193876 [details] did not pass win-ews (win):
Output: http://webkit-commit-queue.appspot.com/results/17191399

[Rouslan Solomakhin]

(In reply to comment #25)
> (From update of attachment 193876 [details])
> Attachment 193876 [details] did not pass mac-wk2-ews (mac-wk2):
> Output: http://webkit-commit-queue.appspot.com/results/17189688
> 
> New failing tests:
> editing/spelling/spellcheck-paste-continuous-disabled.html

Is there a way to extract the test diff from /Volumes/Data/EWS/WebKit/WebKitBuild/Release/layout-test-results/editing/spelling/spellcheck-paste-continuous-disabled-diff.txt on the EWS bot?

[Rouslan Solomakhin]

(In reply to comment #27)
> (In reply to comment #25)
> > (From update of attachment 193876 [details] [details])
> > Attachment 193876 [details] [details] did not pass mac-wk2-ews (mac-wk2):
> > Output: http://webkit-commit-queue.appspot.com/results/17189688
> > 
> > New failing tests:
> > editing/spelling/spellcheck-paste-continuous-disabled.html
> 
> Is there a way to extract the test diff from /Volumes/Data/EWS/WebKit/WebKitBuild/Release/layout-test-results/editing/spelling/spellcheck-paste-continuous-disabled-diff.txt on the EWS bot?

Nevermind. I ran mac-wk2 tests on my machine and found that the new tests fails. This test is based on a test that is skipped in mac-wk2 because of lack of eventSender implementation.  Going to skip the new test on mac-wk2 as well.

[Rouslan Solomakhin]

Created attachment 193905 [details]
Patch

[Rouslan Solomakhin]

Comment on attachment 193905 [details]
Patch

Waiting for EWS bot results.

[Build Bot]

Comment on attachment 193905 [details]
Patch

Attachment 193905 [details] did not pass win-ews (win):
Output: http://webkit-commit-queue.appspot.com/results/17184420

[Rouslan Solomakhin]

Created attachment 193952 [details]
Patch

[Rouslan Solomakhin]

Comment on attachment 193952 [details]
Patch

Added the export symbols for win port. Waiting for EWS results.

[Rouslan Solomakhin]

Comment on attachment 193952 [details]
Patch

All bots are green. Let's commit?

[WebKit Review Bot]

Comment on attachment 193952 [details]
Patch

Clearing flags on attachment: 193952

Committed r146361: <http://trac.webkit.org/changeset/146361>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.