[Webkit-unassigned] [Bug 57088] Refactoring: Editor::TextCheckingOptions should be replaced with TextCheckingType

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=57088





--- Comment #19 from Ryosuke Niwa <rniwa at webkit.org>  2011-05-03 09:35:59 PST ---
(From update of attachment 90281)
View in context: https://bugs.webkit.org/attachment.cgi?id=90281&action=review

> Source/WebCore/editing/Editor.cpp:1981
> +                                 && (isAutomaticQuoteSubstitutionEnabled()

I don't think we do these deep indentation.  Ditto about other places.

> Source/WebCore/editing/Editor.cpp:3259
> +    if (types.isReplacement()) {
> +        result.setLink(isAutomaticLinkDetectionEnabled());
> +        result.setQuote(isAutomaticQuoteSubstitutionEnabled());
> +        result.setDash(isAutomaticDashSubstitutionEnabled());
> +        result.setReplacement(isAutomaticTextReplacementEnabled());
> +        result.setCorrection(types.isSpelling() && isAutomaticSpellingCorrectionEnabled());
>      }

It seems silly that we have to copy flags manually like this.  Can't we make this function a member of TextCheckingTypes and use bitwise operations?  Also, it's hard to verify that adding Correction to TypeCheckingTypes doesn't regress anything.  Could you explain briefly why it's okay?

> Source/WebCore/platform/text/TextChecking.h:54
> +        Correction  = 1 << 9,

This is a new value.  I can't tell easily from the patch whether this is correct or not.

> Source/WebKit/mac/WebCoreSupport/WebEditorClient.mm:860
> +static unsigned toNSTypes(const TextCheckingTypes& types)

Don't they normally name these kinds of functions kit?

> Source/WebKit2/WebProcess/WebCoreSupport/mac/WebEditorClientMac.mm:241
> +static unsigned toNSTypes(const TextCheckingTypes& types)

Ditto.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.