[Webkit-unassigned] [Bug 55571] [Refactoring] Auto correction panel should be handled by its own class.

[bugzilla-daemon at webkit.org]

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





--- Comment #16 from Ryosuke Niwa <rniwa at webkit.org>  2011-03-17 01:12:19 PST ---
(From update of attachment 86040)
View in context: https://bugs.webkit.org/attachment.cgi?id=86040&action=review

>> Source/WebCore/editing/Editor.cpp:1058
>> +    , m_spellingCorrector(SpellingCorrectionController::create(frame))
> 
> Can we have if-def here instead?  It seems odd that create() returns 0 if correction controller is not supported.

On second thought, maybe it's better to have all if-defs in one file.  I'd like to hear other contributors' opinions here.

> Source/WebCore/editing/SpellingCorrectionController.cpp:78
> +SpellingCorrectionController* SpellingCorrectionController::create(Frame* frame)
> +{
> +#if SUPPORT_AUTOCORRECTION_PANEL
> +    return new SpellingCorrectionController(frame);
> +#else
> +    UNUSED_PARAM(frame);
> +    return 0;
> +#endif
> +}

This function should return PassOwnPtr and you should be calling adoptPtr(new SpellingCorrectionController(frame)).  I'm still not happy about the fact all these member functions will be linked in non-Lion WebKit even though they're never called in practice because they are referenced in Editor.cpp.  You should really consider adding EmptySpellingCorrectionController or come up with a clever solution to avoid this.

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