Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 366010

Summary:

[Button] Checkbox does not prevent Space key from being propagated to parent widget

Product:

[RT] RAP

Reporter:

Rüdiger Herrmann <ruediger.herrmann>

Component:

RWT

Assignee:

Project Inbox <rap-inbox>

Status:

RESOLVED FIXED

QA Contact:

Severity:

normal

Priority:

Version:

unspecified

Target Milestone:

1.5 M4

Hardware:

All

OS:

All

Whiteboard:

Attachments:

Description	Flags
Project to reproduce this bug	none
Proposed patch against CVS HEAD	none

Description Rüdiger Herrmann

2011-12-08 06:53:57 EST

To reproduce, run the attached project:
* select a cell from the "Check" column
* hit the space key
-> the check box changes its selection state as expected but also the table item changes its checked state

Reproducible with HEAD and tree_table_merge branch.
This issue only occurs in FF (version 8), Chrome 15 and IE 9 are not affected.

Comment 1 Rüdiger Herrmann

2011-12-08 06:54:26 EST

Created attachment 208088 [details]
Project to reproduce this bug

Comment 2 Ivan Furnadjiev

2011-12-08 07:11:23 EST

Probably related to the fact that when key listener is attached on the server, the DOM key event is always canceled first on the client and than re-fired programatically if it is allowed from the server. Should be interesting if it is reproducible with simple Table with a check box over an item - no TableViewer, no key events involved.

Comment 3 Rüdiger Herrmann

2011-12-08 07:18:54 EST

(In reply to comment #2)
> Probably related to the fact that when key listener is attached on the server,
> the DOM key event is always canceled first on the client and than re-fired
> programatically if it is allowed from the server. Should be interesting if it is
> reproducible with simple Table with a check box over an item - no TableViewer,
> no key events involved.
Though your theory sounds reasonable, the behavior is also reproducible with a check box within a simple table. I added these lines to the TableTab:
    Button button = new Button( table, SWT.CHECK );
    button.setText( "TEST BUTTON ON A TABLE " );
    button.setLocation( 40, 40 );
    button.pack();
Hitting the space key on the check box also changes the checked state of the table item.

Comment 4 Ivan Furnadjiev

2011-12-08 07:54:55 EST

Created attachment 208091 [details]
Proposed patch against CVS HEAD

In BasicButton.js the key event for Enter/Space has been stopped from propagation for "keypress" DOM event too (like "keyup"/"keydown") which is used by Tree/Table.

Comment 5 Rüdiger Herrmann

2011-12-08 08:48:22 EST

Thanks for the patch, Ivan. I applied it to the Tree_Table_Merge branch.

I think events for the Enter key should also be not propagated as - like the Space key - they are handled by the BasicButton itself. What do you think?

Comment 6 Ivan Furnadjiev

2011-12-08 08:57:14 EST

(In reply to comment #5)
> I think events for the Enter key should also be not propagated as - like the
> Space key - they are handled by the BasicButton itself. What do you think?
Yes, I think so too... and stop propagation of Enter key event is also included in the patch. :-)

Comment 7 Rüdiger Herrmann

2011-12-08 09:21:14 EST

(In reply to comment #6)
> (In reply to comment #5)
> > I think events for the Enter key should also be not propagated as - like the
> > Space key - they are handled by the BasicButton itself. What do you think?
> Yes, I think so too... and stop propagation of Enter key event is also included
> in the patch. :-)
Sorry, I must have missed this somehow.
Applied patch to HEAD, too.