Bug 118739 – REGRESSION (r150291): Chinese predictive input pop-up disappears on twitter.com

• Component: HTML Editing
• Status: RESOLVED FIXED

[Alexey Proskuryakov]

Steps to reproduce:

1. Enable Cangjie (Traditional Chinese) input in System Preferences > Keyboard > Input Sources.
2. In the settings for Cangjie, make sure "Show prediction candidates" is turned on.
3. Switch to the Cangjie input source.
4. Click Reply on a tweet at twitter.com.
5. Type "hqi<space>.

Results: a predictive input pop-up momentarily appears, but does not stay on screen.

<rdar://problem/14300350>

[Alexey Proskuryakov]

Created attachment 206808 [details]
proposed fix

I couldn't make a test for this issue. WebKitTestRunner tests from WebProcess side, and there were too many timing issues trying to detect an effect of this patch from there.

[Daniel Bates]

Comment on attachment 206808 [details]
proposed fix

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

r=me

> Source/WebKit2/UIProcess/mac/WebPageProxyMac.mm:224
> +    m_temporarilyClosedComposition = false;

I'm unclear if you're unhappy about this addition given your ChangeLog remark on it being a "targeted fix just for this specific [bug]". If you're unhappy about this change then I suggest we add a FIXME comment and/or file a bug (and reference it in the FIXME comment) so that we may revisit this fix should we come up with a better approach at a later date.

On another note, I noticed that WebPageProxy::m_temporarilyClosedComposition is only used in the Apple Mac WebKit port (as of the time of writing). We should consider guarding it with an #idef so as to make it clear at the call site that it's specific to the Apple Mac WebKit port. Obviously we should look to better encapsulate such Mac-specific implementation details so as to minimize the use of platform-specific #ifdefs in platform-independent files.

[Alexey Proskuryakov]

Committed <http://trac.webkit.org/r152740>.

> If you're unhappy about this change then I suggest we add a FIXME comment and/or file a bug

This specific change should be perfectly good. What is not good is that other similar places don't reset m_temporarilyClosedComposition, and that if I change them to do that, then things break. EditorState tracking is quite unreliable in general.