Opened 8 years ago

Closed 8 years ago

#4677 closed Bug (fixed)

Keyboard navigation problem with tabs in image dialog

Reported by: Damian Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.1
Component: Accessibility Version: SVN (CKEditor) - OLD
Keywords: Confirmed IBM Review+ Cc:

Description

To Reproduce the Issue on the Link Tab.

Open the CK Editor.
Access the Tool bar by Pressing(ALT+F10)
Navigate to Insert Image Icon.
Press Enter.
on the Insert Image Light Box Navigate to Link Tab by Pressing Alt+F10 and then Press Tab
Press Enter.

Actual result: Focus is going no where..The User has to Press Tab twice in order for the Focus to go back in to the First Field.which is URL Text Field.so JAWS is not Reading any thing until the User Press the Tab Key Twice..This will confuse the User whether he is on the Light Box..or the Focus has gone out of the Window.

This will also happen when you Reach the Cancel Link and then we have to Press Tab Twice in order for the Focus to go back to URL Text Field..

Expected result: Focus should go to the URL Text Field and JAWS should Read URL Edit Type and Text..so the User can Understand that there is an Input Text box.

Attachments (3)

4677.patch (3.7 KB) - added by Garry Yao 8 years ago.
4677_2.patch (2.5 KB) - added by Garry Yao 8 years ago.
4677_3.patch (2.5 KB) - added by Garry Yao 8 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 8 years ago by Garry Yao

Keywords: Confirmed added
Version: SVN (CKEditor)

Changed 8 years ago by Garry Yao

Attachment: 4677.patch added

comment:2 Changed 8 years ago by Garry Yao

Keywords: Review? added
Owner: set to Garry Yao
Status: newassigned

Similar with #4542.

comment:3 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

The fix works well. The only issue is that we already have isVisible in CKEDITOR.dom.element. It should be verified and eventually amended, if needed.

comment:4 Changed 8 years ago by Frederico Caldeira Knabben

Btw... there are test files in the patch that are not anymore available on trunk.

Changed 8 years ago by Garry Yao

Attachment: 4677_2.patch added

comment:5 Changed 8 years ago by Garry Yao

Keywords: Review? added; Review- removed

comment:6 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

There are extra curly braces being added to the isVisible function. Surprisingly it works, but it must be fixed.

Other than that, we may fix the implementation to make it perform a bit better:

isVisible : function()
{
	var isVisible = !!this.$.offsetHeight && this.getComputedStyle( 'visibility' ) != 'hidden',
		elementWindow;

	// Webkit and Opera report non-zero offsetHeight despite that
	// element is inside an invisible iframe. (#4542)
	if ( isVisible && ( CKEDITOR.env.webkit || CKEDITOR.env.opera ) )
	{
		elementWindow = this.getWindow();
		
		if ( !elementWindow.equals( CKEDITOR.document.getWindow() )
			&& ( elementWindow = elementWindow.$.frameElement ) )
		{
			isVisible = new CKEDITOR.dom.element( elementWindow ).isVisible();
		}
	}

	return isVisible;
}

Changed 8 years ago by Garry Yao

Attachment: 4677_3.patch added

comment:7 Changed 8 years ago by Garry Yao

Keywords: Review? added; Review- removed

It seems that I really need to lint before patching these days ;)

comment:8 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

Please fix coding style for the if block at line 752 before committing.

comment:9 Changed 8 years ago by Garry Yao

Resolution: fixed
Status: assignedclosed

Fixed with [4534].

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