[webkit-reviews] review denied: [Bug 112362] Move setAsynchronousSpellCheckingEnabled to internals.settings : [Attachment 193148] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Mar 14 10:55:57 PDT 2013


Tony Chang <tony at chromium.org> has denied  review:
Bug 112362: Move setAsynchronousSpellCheckingEnabled to internals.settings
https://bugs.webkit.org/show_bug.cgi?id=112362

Attachment 193148: Patch
https://bugs.webkit.org/attachment.cgi?id=193148&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=193148&action=review


> Source/WebCore/testing/InternalSettings.cpp:78
> +    ,
m_originalUnifiedSpellCheckerEnabled(settings->unifiedTextCheckerEnabled())
> +    ,
m_originalAsynchronousSpellCheckingEnabled(settings->asynchronousSpellCheckingE
nabled())

You don't need to add this.  The generated code from Settings.in already does
the backup/restore for you.

> Source/WebCore/testing/InternalSettings.cpp:117
> +   
settings->setUnifiedTextCheckerEnabled(m_originalUnifiedSpellCheckerEnabled);
> +   
settings->setAsynchronousSpellCheckingEnabled(m_originalAsynchronousSpellChecki
ngEnabled);

You should be able to delete this too.

> Source/WebCore/testing/InternalSettings.h:64
>	   bool m_originalUnifiedSpellCheckerEnabled;
> +	   bool m_originalAsynchronousSpellCheckingEnabled;

And be able to delete these too.

> LayoutTests/editing/spelling/grammar-markers-hidpi.html:30
> +	   if (window.internals)
> +	       internals.settings.setAsynchronousSpellCheckingEnabled(false);

Is this trying to reset the value?  The test harness should do that
automatically and we shouldn't need this code.	Can you try removing this and
seeing if tests that run afterwards fail?

> LayoutTests/editing/spelling/grammar-markers.html:27
> +	   if (window.internals)
> +	       internals.settings.setAsynchronousSpellCheckingEnabled(false);

Ditto.

> LayoutTests/editing/spelling/grammar-paste.html:52
> +    internals.settings.setAsynchronousSpellCheckingEnabled(false);

Ditto.

> LayoutTests/editing/spelling/script-tests/spellcheck-paste.js:43
> +    internals.settings.setAsynchronousSpellCheckingEnabled(false);

Ditto.

> LayoutTests/editing/spelling/spellcheck-async-mutation.html:82
> +    if (window.internals) {
>	   internals.settings.setUnifiedTextCheckerEnabled(false);
> +	   internals.settings.setAsynchronousSpellCheckingEnabled(false);
> +    }

You should be able to remove this whole block.

> LayoutTests/editing/spelling/spellcheck-async.html:63
> +    if (window.internals) {
>	   internals.settings.setUnifiedTextCheckerEnabled(false);
> +	   internals.settings.setAsynchronousSpellCheckingEnabled(false);
> +    }

You should be able to remove this whole block.

> LayoutTests/editing/spelling/spellcheck-paste-disabled.html:54
> +    internals.settings.setAsynchronousSpellCheckingEnabled(false);

Remove.

> LayoutTests/editing/spelling/spellcheck-queue.html:84
> +    if (window.internals)
> +	   internals.settings.setAsynchronousSpellCheckingEnabled(false);

Remove.

> LayoutTests/editing/spelling/spellcheck-sequencenum.html:81
> +    if (window.internals) {
> +	   internals.settings.setAsynchronousSpellCheckingEnabled(false);

Remove.

> LayoutTests/editing/spelling/spelling-marker-description.html:15
> +    internals.settings.setAsynchronousSpellCheckingEnabled(false);

Remove.

> LayoutTests/platform/chromium/editing/spelling/delete-misspelled-word.html:40

> +    internals.settings.setAsynchronousSpellCheckingEnabled(false);

Remove.

>
LayoutTests/platform/chromium/editing/spelling/move-cursor-to-misspelled-word.h
tml:21
> +internals.settings.setAsynchronousSpellCheckingEnabled(false);

Remove.


More information about the webkit-reviews mailing list