Bug 86200 – [EFL] fast/events/arrow-keys-on-body.html does not pass

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebKit EFL

[Mikhail Pozdnyakov]

fast/events/arrow-keys-on-body.html wrong test result, keypress should not be emitted.

[Mikhail Pozdnyakov]

Keypress event should be raised by noncharacter keys.

[Mikhail Pozdnyakov]

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)]

LGTM.

[Gyuyoung Kim]

Comment on attachment 141397 [details]
Fix. 

Looks fine too.

[WebKit Review Bot]

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]

Created attachment 142247 [details]
to be landed.

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 142247 [details]
to be landed.

Hmm. Can you check if your change is still needed after r117218?

[Mikhail Pozdnyakov]

(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)]

(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]

(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]

> 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)]

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)]

Closing for now -- if we do find some problem in the future, we can revisit this patch.