Opened 10 years ago

Closed 10 years ago

#5450 closed Bug (fixed)

Press enter on 'replace' button result wrong

Reported by: Garry Yao Owned by: brooks
Priority: Normal Milestone: CKEditor 3.3
Component: UI : Dialogs Version:
Keywords: Review+ Cc:

Description

Reproducing Procedures

  1. Load the any of the sample page;
  2. Open 'Replace' dialog and fill in 'are';
  3. Tab to focus the 'Replace All' button and press 'Enter';
  • Expected Result: You get only the success message.
  • Actual Result: You get both message: 'Specified text not found' and '1 occurance replaced'.

Attachments (3)

5450.patch (947 bytes) - added by brooks 10 years ago.
5450_2.patch (787 bytes) - added by brooks 10 years ago.
5450_3.patch (532 bytes) - added by brooks 10 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 10 years ago by brooks

Owner: set to brooks
Status: newassigned

Changed 10 years ago by brooks

Attachment: 5450.patch added

comment:2 Changed 10 years ago by brooks

Keywords: Review? added; Confirmed removed

this bug only happen in firefox, it's a weired behavior

when debugger, e.g, add break point in source code, u can't reproduce the bug

I trace to here:
_source/plugins/dialogui/plugin.js#503

element.on( 'click', function( evt )
{   console.log( 'click fired!' );
	me.fire( 'click', { dialog : me.getDialog() } );
	evt.data.preventDefault();
} );

element.on( 'keydown', function( evt )
{
	if ( evt.data.getKeystroke() in { 32:1, 13:1 } )
	{	console.log( 'keydown fired!' );
		me.click();
		evt.data.preventDefault();
	}
} );

then found when hit enter,the click event fired too.

comment:3 Changed 10 years ago by Garry Yao

Keywords: Review- added; Review? removed

this bug only happen in firefox, it's a weired behavior

It's indeed a Firefox bug, I believe the default event behavior is somehow affected by the focus (alert box grab the focus from button). While the real problem is that the [http://dev.fckeditor.net/browser/CKEditor/trunk/_source/plugins/dialogui/plugin.js#L507

click simulating logics] are not needed at all, since most major browsers and assistive technologies trigger onClick if the Enter key is pressed when the link has focus.

Changed 10 years ago by brooks

Attachment: 5450_2.patch added

comment:4 Changed 10 years ago by brooks

Keywords: Review? added; Review- removed

yes, in firefox, when a link is focused and then hit enter key, it will fire click event and couldn't be prevent.

comment:5 Changed 10 years ago by Garry Yao

Keywords: Review- added; Review? removed

It's not a FF only behavior, actual all browsers agree with it on link, and our buttons are all made of link, so IMO we just need to remove the enterkey delegation in all places:

if ( evt.data.getKeystroke() in { 32:1 } )

Changed 10 years ago by brooks

Attachment: 5450_3.patch added

comment:6 Changed 10 years ago by brooks

Keywords: Review? added; Review- removed

the FF only behavior is that when a link is focused,the enterkey delegation couldn't be prevented,but other browsers could.

of course,it's a nice idea not to listen the keydown event for enter key in this case, 'cause the click event will always fired when hit a focused link.

comment:7 Changed 10 years ago by Garry Yao

Keywords: Review+ added; Review? removed

Please remove the extra spaces on committing.

comment:8 Changed 10 years ago by brooks

Resolution: fixed
Status: assignedclosed

fixed with 5402 5402

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy