Opened 10 years ago

Closed 10 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 10 years ago.
4677_2.patch (2.5 KB) - added by Garry Yao 10 years ago.
4677_3.patch (2.5 KB) - added by Garry Yao 10 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 10 years ago by Garry Yao

Keywords: Confirmed added
Version: SVN (CKEditor)

Changed 10 years ago by Garry Yao

Attachment: 4677.patch added

comment:2 Changed 10 years ago by Garry Yao

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

Similar with #4542.

comment:3 Changed 10 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 10 years ago by Frederico Caldeira Knabben

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

Changed 10 years ago by Garry Yao

Attachment: 4677_2.patch added

comment:5 Changed 10 years ago by Garry Yao

Keywords: Review? added; Review- removed

comment:6 Changed 10 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 10 years ago by Garry Yao

Attachment: 4677_3.patch added

comment:7 Changed 10 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 10 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 10 years ago by Garry Yao

Resolution: fixed
Status: assignedclosed

Fixed with [4534].

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