Bug 118645 – [WK2][GTK] Enable asynchronous spell checking by default

• Status: NEW
• Component: WebKitGTK

[Kwang Yul Seo]

Asynchronous spell checking is disabled by default (Bug 81042). It means all requests of checking the string are sent synchronously.

Once requestCheckingOfString method is added (Bug 118644), we can perform spelling checking asynchronously. We should enable it by default for GTK to use non blocking API.

[Martin Robinson]

Created attachment 252361 [details]
Patch

[Martin Robinson]

Whoops. Uploaded this patch to the wrong bug.

[Darin Adler]

Comment on attachment 252361 [details]
Patch

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

> Source/WebCore/platform/graphics/Font.cpp:338
> +#if PLATFORM(COCOA) || USE(CAIRO)

This is such a mysterious combination. It would be nice to have some named thing that makes it clear what these have in common. Perhaps they are the only platforms that support synthetic obliquing?

> Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp:147
> +    ASSERT(m_pattern);

The right way to do this in modern WebKit coding style is to change the argument from FcPattern* to FcPattern&.

[Martin Robinson]

(In reply to comment #3)
> Comment on attachment 252361 [details]
> Patch

Thanks for the review!  I have to admit though that uploading the patch here was a mistake. It should have gone to https://bugs.webkit.org/show_bug.cgi?id=144612 originally.

> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=252361&action=review
> 
> > Source/WebCore/platform/graphics/Font.cpp:338
> > +#if PLATFORM(COCOA) || USE(CAIRO)
> 
> This is such a mysterious combination. It would be nice to have some named
> thing that makes it clear what these have in common. Perhaps they are the
> only platforms that support synthetic obliquing?

As far as I can tell, the original #ifdef was due to the fact that other platforms did not support creating explicitly non-synthetic-oblique fonts. It may be worth simply stubbing out the implementation on Windows and removing the #ifdef. I'll try that in a followup patch.

> 
> > Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp:147
> > +    ASSERT(m_pattern);
> 
> The right way to do this in modern WebKit coding style is to change the
> argument from FcPattern* to FcPattern&.

Is that still an option given that FcPattern* is an opaque pointer?

[Martin Robinson]

(In reply to comment #4)
> (In reply to comment #3)
> > Comment on attachment 252361 [details]
> > Patch
> 
> Thanks for the review!  I have to admit though that uploading the patch here
> was a mistake. It should have gone to
> https://bugs.webkit.org/show_bug.cgi?id=144612 originally.

Hopefully you don't mind if I simply land the version with the correct ChangeLog? Sorry for the mixup.