Bug 158227

Summary: REGRESSION(r191907): Can't enter combining diacritic marks in Web Inspector fields
Product: WebKit Reporter: BJ Burg <bburg>
Component: WebKit2Assignee: BJ Burg <bburg>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, ap, bburg, buildbot, darin, enrica, rniwa, thorton, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Proposed Fix
none
Archive of layout-test-results from ews107 for mac-yosemite-wk2
none
Proposed Fix (with Test) ap: review+, ap: commit-queue-

Description BJ Burg 2016-05-31 10:14:44 PDT
Oops!
Comment 1 BJ Burg 2016-05-31 10:14:52 PDT
<rdar://problem/26232464>
Comment 2 BJ Burg 2016-05-31 10:58:33 PDT
Created attachment 280156 [details]
Proposed Fix
Comment 3 Tim Horton 2016-05-31 11:01:27 PDT
Comment on attachment 280156 [details]
Proposed Fix

It seems like this should be API-testable?
Comment 4 Build Bot 2016-05-31 11:37:16 PDT
Comment on attachment 280156 [details]
Proposed Fix

Attachment 280156 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/1413508

Number of test failures exceeded the failure limit.
Comment 5 Build Bot 2016-05-31 11:37:19 PDT
Created attachment 280160 [details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 6 BJ Burg 2016-05-31 11:53:07 PDT
(In reply to comment #3)
> Comment on attachment 280156 [details]
> Proposed Fix
> 
> It seems like this should be API-testable?

It probably is, but I was hesitant to write one since the simulated NSEvents to send will depend on the system keyboard layout. I don't think we can assume a specific keyboard layout?

If this isn't an issue, I would have to postpone writing a test until later this week at least. It could take a bit of time to get it right since we don't have any other WKWebView tests that simulate key events.
Comment 7 Alexey Proskuryakov 2016-05-31 12:11:39 PDT
Is this bug somehow specific to Web Inspector, or are all WKWebViews affected?

Simulating raw keydowns to have them translated by the OS would be wrong, as you correctly suggested. But simply verifying that -inputContext returns non-null when the web view has editable text focused would be a pretty solid regression test.
Comment 8 Darin Adler 2016-06-01 09:01:56 PDT
(In reply to comment #6)
> (In reply to comment #3)
> It probably is, but I was hesitant to write one since the simulated NSEvents
> to send will depend on the system keyboard layout. I don't think we can
> assume a specific keyboard layout?
> 
> If this isn't an issue, I would have to postpone writing a test until later
> this week at least. It could take a bit of time to get it right since we
> don't have any other WKWebView tests that simulate key events.

Those reasons don’t seem to be sufficient reasons not to write a test now. A test that works only with US keyboard layout is likely OK.
Comment 9 BJ Burg 2016-06-01 11:08:33 PDT
(In reply to comment #8)
> (In reply to comment #6)
> > (In reply to comment #3)
> > It probably is, but I was hesitant to write one since the simulated NSEvents
> > to send will depend on the system keyboard layout. I don't think we can
> > assume a specific keyboard layout?
> > 
> > If this isn't an issue, I would have to postpone writing a test until later
> > this week at least. It could take a bit of time to get it right since we
> > don't have any other WKWebView tests that simulate key events.
> 
> Those reasons don’t seem to be sufficient reasons not to write a test now. A
> test that works only with US keyboard layout is likely OK.

I will write the regression test as proposed by Alexey as it's very straightforward and should cover the same issue.
Comment 10 BJ Burg 2016-06-01 20:38:09 PDT
Created attachment 280302 [details]
Proposed Fix (with Test)
Comment 11 Alexey Proskuryakov 2016-06-01 21:18:07 PDT
Comment on attachment 280302 [details]
Proposed Fix (with Test)

r=me with the test disabled on iOS.
Comment 12 BJ Burg 2016-06-01 21:27:59 PDT
(In reply to comment #11)
> Comment on attachment 280302 [details]
> Proposed Fix (with Test)
> 
> r=me with the test disabled on iOS.

Oops, OK.
Comment 13 BJ Burg 2016-06-01 22:20:13 PDT
Committed r201592: <http://trac.webkit.org/changeset/201592>