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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Apr 7 11:23:05 PDT 2011


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





--- Comment #8 from MORITA Hajime <morrita at google.com>  2011-04-07 11:23:05 PST ---
Hi Ryosuke, thank you for taking a look!
I update the patch to your feedback.

(In reply to comment #5)
> (From update of attachment 88562 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=88562&action=review
> 
> > Source/WebCore/editing/Editor.cpp:2226
> > +    bool shouldMarkSpelling = textCheckingTypes.spelling();
> > +    bool shouldMarkGrammar = textCheckingTypes.grammar();
> > +    bool shouldPerformReplacement = textCheckingTypes.replacement();
> > +    bool shouldShowCorrectionPanel = textCheckingTypes.correctionPanel();
> > +    bool shouldCheckForCorrection = shouldShowCorrectionPanel || textCheckingTypes.correction();
> 
> I think these member functions should be named as isSpelling, isGrammar, etc...
Fixed.

> 
> > Source/WebCore/editing/Editor.cpp:3623
> > -TextCheckingTypeMask Editor::textCheckingTypeMaskFor(TextCheckingOptions textCheckingOptions)
> > +TextCheckingTypes Editor::addTextCheckSettings(const TextCheckingTypes& textCheckingTypes)
> 
> I don't think this function name makes sense since you're not modifying the original textCheckingTypes.  Something like resolveReplacedTextCheckingType might work.
Fixed.

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



More information about the webkit-unassigned mailing list