Bug 162921 - Add support for KeyboardEvent.isComposing attribute
Summary: Add support for KeyboardEvent.isComposing attribute
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: WebExposed
Depends on:
Blocks: 162852
  Show dependency treegraph
 
Reported: 2016-10-04 10:51 PDT by Chris Dumez
Modified: 2016-10-04 17:05 PDT (History)
10 users (show)

See Also:


Attachments
WIP patch (3.11 KB, patch)
2016-10-04 10:59 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP Patch (3.13 KB, patch)
2016-10-04 11:01 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (12.04 KB, patch)
2016-10-04 11:58 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (12.72 KB, patch)
2016-10-04 16:26 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (12.31 KB, patch)
2016-10-04 16:36 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2016-10-04 10:51:10 PDT
Add support for KeyboardEvent.isComposing attribute:
- https://www.w3.org/TR/uievents/#dom-keyboardevent-iscomposing
- https://www.w3.org/TR/uievents/#ref-for-dom-keyboardevent-iscomposing-3
Comment 1 Chris Dumez 2016-10-04 10:57:11 PDT
Manual test app:
- https://jsfiddle.net/ay92sr08/8/

Tested with Chinese input method (Pinyin):
[Log] compositionstart (show, line 46)
[Log] keydown: n, isComposing: true (show, line 52)
[Log] keyup: n, isComposing: true (show, line 55)
[Log] keydown: i, isComposing: true (show, line 52)
[Log] keyup: i, isComposing: true (show, line 55)
[Log] compositionend (show, line 49)
Comment 2 Chris Dumez 2016-10-04 10:59:39 PDT
Created attachment 290621 [details]
WIP patch
Comment 3 Chris Dumez 2016-10-04 11:01:25 PDT
Created attachment 290623 [details]
WIP Patch
Comment 4 Chris Dumez 2016-10-04 11:36:09 PDT
Updated manual test case:
https://jsfiddle.net/ay92sr08/9/

Result:
[Log] compositionstart (show, line 46)
[Log] compositionupdate (show, line 52, x2)
[Log] keydown: n, isComposing: true (show, line 55)
[Log] keyup: n, isComposing: true (show, line 58)
[Log] compositionupdate (show, line 52, x2)
[Log] keydown: i, isComposing: true (show, line 55)
[Log] keyup: i, isComposing: true (show, line 58)
[Log] compositionend (show, line 49)
Comment 5 Chris Dumez 2016-10-04 11:58:49 PDT
Created attachment 290630 [details]
Patch
Comment 6 Darin Adler 2016-10-04 16:04:35 PDT
Comment on attachment 290630 [details]
Patch

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

> Source/WebCore/dom/KeyboardEvent.cpp:117
> +    , m_isComposing(view && view->frame() ? view->frame()->editor().hasComposition() : false)

I think you should just use another && here instead of using ? :

> Source/WebCore/dom/KeyboardEvent.h:130
> +    bool m_isComposing : 1;

Maybe the old bool item should be changed to not use bitfields instead?
Comment 7 Chris Dumez 2016-10-04 16:26:19 PDT
Created attachment 290664 [details]
Patch
Comment 8 Chris Dumez 2016-10-04 16:36:30 PDT
Created attachment 290667 [details]
Patch
Comment 9 WebKit Commit Bot 2016-10-04 17:05:18 PDT
Comment on attachment 290667 [details]
Patch

Clearing flags on attachment: 290667

Committed r206796: <http://trac.webkit.org/changeset/206796>
Comment 10 WebKit Commit Bot 2016-10-04 17:05:23 PDT
All reviewed patches have been landed.  Closing bug.