RESOLVED FIXED 158227
REGRESSION(r191907): Can't enter combining diacritic marks in Web Inspector fields
https://bugs.webkit.org/show_bug.cgi?id=158227
Summary REGRESSION(r191907): Can't enter combining diacritic marks in Web Inspector f...
Blaze Burg
Reported 2016-05-31 10:14:44 PDT
Oops!
Attachments
Proposed Fix (2.29 KB, patch)
2016-05-31 10:58 PDT, Blaze Burg
no flags
Archive of layout-test-results from ews107 for mac-yosemite-wk2 (893.88 KB, application/zip)
2016-05-31 11:37 PDT, Build Bot
no flags
Proposed Fix (with Test) (11.70 KB, patch)
2016-06-01 20:38 PDT, Blaze Burg
ap: review+
ap: commit-queue-
Blaze Burg
Comment 1 2016-05-31 10:14:52 PDT
Blaze Burg
Comment 2 2016-05-31 10:58:33 PDT
Created attachment 280156 [details] Proposed Fix
Tim Horton
Comment 3 2016-05-31 11:01:27 PDT
Comment on attachment 280156 [details] Proposed Fix It seems like this should be API-testable?
Build Bot
Comment 4 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.
Build Bot
Comment 5 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
Blaze Burg
Comment 6 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.
Alexey Proskuryakov
Comment 7 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.
Darin Adler
Comment 8 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.
Blaze Burg
Comment 9 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.
Blaze Burg
Comment 10 2016-06-01 20:38:09 PDT
Created attachment 280302 [details] Proposed Fix (with Test)
Alexey Proskuryakov
Comment 11 2016-06-01 21:18:07 PDT
Comment on attachment 280302 [details] Proposed Fix (with Test) r=me with the test disabled on iOS.
Blaze Burg
Comment 12 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.
Blaze Burg
Comment 13 2016-06-01 22:20:13 PDT
Note You need to log in before you can comment on or make changes to this bug.