RESOLVED WONTFIX 86200
[EFL] fast/events/arrow-keys-on-body.html does not pass
https://bugs.webkit.org/show_bug.cgi?id=86200
Summary [EFL] fast/events/arrow-keys-on-body.html does not pass
Mikhail Pozdnyakov
Reported 2012-05-11 05:44:11 PDT
fast/events/arrow-keys-on-body.html wrong test result, keypress should not be emitted.
Attachments
Fix. (2.86 KB, patch)
2012-05-11 06:25 PDT, Mikhail Pozdnyakov
gustavo: review+
webkit.review.bot: commit-queue-
to be landed. (2.99 KB, patch)
2012-05-16 06:59 PDT, Mikhail Pozdnyakov
no flags
Mikhail Pozdnyakov
Comment 1 2012-05-11 06:07:56 PDT
Keypress event should be raised by noncharacter keys.
Mikhail Pozdnyakov
Comment 2 2012-05-11 06:25:34 PDT
Created attachment 141397 [details] Fix. Non-characters keys have empty text value. This patch should be applied after bug85503 is landed, as bug85503 fixes windowsKeyMap population.
Raphael Kubo da Costa (:rakuco)
Comment 3 2012-05-11 06:47:33 PDT
LGTM.
Gyuyoung Kim
Comment 4 2012-05-13 19:18:58 PDT
Comment on attachment 141397 [details] Fix. Looks fine too.
WebKit Review Bot
Comment 5 2012-05-16 06:50:40 PDT
Comment on attachment 141397 [details] Fix. Rejecting attachment 141397 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: fset -113 lines). patching file Source/WebCore/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WebCore/platform/efl/EflKeyboardUtilities.cpp Hunk #1 succeeded at 202 (offset 12 lines). Hunk #2 FAILED at 215. 1 out of 2 hunks FAILED -- saving rejects to file Source/WebCore/platform/efl/EflKeyboardUtilities.cpp.rej Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Gustavo No..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue/ Full output: http://queues.webkit.org/results/12703792
Mikhail Pozdnyakov
Comment 6 2012-05-16 06:59:01 PDT
Created attachment 142247 [details] to be landed.
Raphael Kubo da Costa (:rakuco)
Comment 7 2012-05-16 07:26:33 PDT
Comment on attachment 142247 [details] to be landed. Hmm. Can you check if your change is still needed after r117218?
Mikhail Pozdnyakov
Comment 8 2012-05-16 07:39:57 PDT
(In reply to comment #7) > (From update of attachment 142247 [details]) > Hmm. Can you check if your change is still needed after r117218? I guess might make sense, r117218 is touching EventSender which is part of DRT. Changes in this bug are part of core where I think the values still should be checked.
Raphael Kubo da Costa (:rakuco)
Comment 9 2012-05-16 08:13:09 PDT
(In reply to comment #8) > (In reply to comment #7) > > Hmm. Can you check if your change is still needed after r117218? > > I guess might make sense, r117218 is touching EventSender which is part of DRT. > Changes in this bug are part of core where I think the values still should be checked. r117218 completely solves the problem when the code path is reached via DRT, right? Then it would be good to check if this is something that can happen via normal user interaction as well, that is, check how ecore_evas initializes the event structure passed to evas -- I can't remember how it initializes the string member off the top of my head.
Mikhail Pozdnyakov
Comment 10 2012-05-16 10:40:40 PDT
(In reply to comment #9) > (In reply to comment #8) > > (In reply to comment #7) > > > Hmm. Can you check if your change is still needed after r117218? > > > > I guess might make sense, r117218 is touching EventSender which is part of DRT. > > Changes in this bug are part of core where I think the values still should be checked. > > r117218 completely solves the problem when the code path is reached via DRT, right? Then it would be good to check if this is something that can happen via normal user interaction as well, that is, check how ecore_evas initializes the event structure passed to evas -- I can't remember how it initializes the string member off the top of my head. from http://docs.enlightenment.org/auto/evas/group__Evas__Event__Feeding__Group.html : void evas_event_feed_key_down (Evas * e, const char * keyname, const char * key, const char * string, const char * compose, unsigned int timestamp, const void * data ) Key down event feed. Parameters: e The canvas to thaw out keyname Name of the key key The key pressed. string A String compose The compose string timestamp Timestamp of the mouse up event data Data for canvas. The situations like evas_event_feed_key_down(evas, "Left", "Left", "some garbage that will turn up in webkit PlatformKeyboardEvent", 0, 0, 0); are quite possible.
Mikhail Pozdnyakov
Comment 11 2012-05-18 01:20:35 PDT
> The situations like > > evas_event_feed_key_down(evas, "Left", "Left", "some garbage that will turn up in webkit PlatformKeyboardEvent", 0, 0, 0); > > are quite possible. And this will lead to appearance of unneeded Keypress events.
Raphael Kubo da Costa (:rakuco)
Comment 12 2012-05-18 07:31:26 PDT
We have discussed this on IRC, and agreed to only unskip the test that is already passing. I've done so in <http://trac.webkit.org/changeset/117591>. The rationale is that right now ecore and evas do not have a cross-backend way of representing keys (like what's present in gdk and Qt), and the current implementation in EflKeyboardUtilities.cpp is only expected to work with the ecore_x xlib backend. We therefore do not expect people to do what was cited in comment #11, and so we have decided not to fix what is not broken.
Raphael Kubo da Costa (:rakuco)
Comment 13 2012-05-18 07:32:15 PDT
Closing for now -- if we do find some problem in the future, we can revisit this patch.
Note You need to log in before you can comment on or make changes to this bug.