Skip to content

Commit

Permalink
Remove empty table cells from the dialog of specialchar plugin (#3566)
Browse files Browse the repository at this point in the history
Remove empty table cells from the dialog of specialchar plugin

Co-authored-by: Tomasz Jakut <[email protected]>
  • Loading branch information
Comandeer authored Nov 4, 2019
2 parents ad38167 + c74ad40 commit 42ff73a
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 55 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Fixed Issues:
* [#3413](https://github.com/ckeditor/ckeditor4/issues/3413): Fixed: Menu items with labels containing double quotes are rendered incorrectly.
* [#3475](https://github.com/ckeditor/ckeditor4/issues/3475): [Firefox] Fixed: Pasting plain text over existing content fails and throws an error.
* [#2027](https://github.com/ckeditor/ckeditor4/issues/2027): Fixed: Incorrect email display text after reopening [Link](https://ckeditor.com/cke4/addon/link) dialog for display names starting with `@`.
* [#3544](https://github.com/ckeditor/ckeditor4/issues/3544): Fixed: [Special Characters](https://ckeditor.com/cke4/addon/specialchar) dialog read incorrectly by screen readers due to empty table cells at the end.

## CKEditor 4.13

Expand Down
113 changes: 58 additions & 55 deletions plugins/specialchar/dialogs/specialchar.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,69 +7,74 @@ CKEDITOR.dialog.add( 'specialchar', function( editor ) {
// Simulate "this" of a dialog for non-dialog events.
// @type {CKEDITOR.dialog}
var dialog,
lang = editor.lang.specialchar;

var onChoice = function( evt ) {
var target, value;
if ( evt.data )
target = evt.data.getTarget();
else
target = new CKEDITOR.dom.element( evt );

if ( target.getName() == 'a' && ( value = target.getChild( 0 ).getHtml() ) ) {
target.removeClass( 'cke_light_background' );
dialog.hide();

// We must use "insertText" here to keep text styled.
var span = editor.document.createElement( 'span' );
span.setHtml( value );
editor.insertText( span.getText() );
}
};

var onClick = CKEDITOR.tools.addFunction( onChoice );
lang = editor.lang.specialchar,
focusedNode,
onChoice,
onClick,
onBlur,
onFocus,
onKeydown;


onChoice = function( evt ) {
var target, value;
if ( evt.data )
target = evt.data.getTarget();
else
target = new CKEDITOR.dom.element( evt );

if ( target.getName() == 'a' && ( value = target.getChild( 0 ).getHtml() ) ) {
target.removeClass( 'cke_light_background' );
dialog.hide();

// We must use "insertText" here to keep text styled.
var span = editor.document.createElement( 'span' );
span.setHtml( value );
editor.insertText( span.getText() );
}
};

var focusedNode;
onClick = CKEDITOR.tools.addFunction( onChoice );

var onFocus = function( evt, target ) {
var value;
target = target || evt.data.getTarget();
onFocus = function( evt, target ) {
var value;
target = target || evt.data.getTarget();

if ( target.getName() == 'span' )
target = target.getParent();
if ( target.getName() == 'span' )
target = target.getParent();

if ( target.getName() == 'a' && ( value = target.getChild( 0 ).getHtml() ) ) {
// Trigger blur manually if there is focused node.
if ( focusedNode )
onBlur( null, focusedNode );
if ( target.getName() == 'a' && ( value = target.getChild( 0 ).getHtml() ) ) {
// Trigger blur manually if there is focused node.
if ( focusedNode )
onBlur( null, focusedNode );

var htmlPreview = dialog.getContentElement( 'info', 'htmlPreview' ).getElement();
var htmlPreview = dialog.getContentElement( 'info', 'htmlPreview' ).getElement();

dialog.getContentElement( 'info', 'charPreview' ).getElement().setHtml( value );
htmlPreview.setHtml( CKEDITOR.tools.htmlEncode( value ) );
target.getParent().addClass( 'cke_light_background' );
dialog.getContentElement( 'info', 'charPreview' ).getElement().setHtml( value );
htmlPreview.setHtml( CKEDITOR.tools.htmlEncode( value ) );
target.getParent().addClass( 'cke_light_background' );

// Memorize focused node.
focusedNode = target;
}
};
// Memorize focused node.
focusedNode = target;
}
};

var onBlur = function( evt, target ) {
target = target || evt.data.getTarget();
onBlur = function( evt, target ) {
target = target || evt.data.getTarget();

if ( target.getName() == 'span' )
target = target.getParent();
if ( target.getName() == 'span' )
target = target.getParent();

if ( target.getName() == 'a' ) {
dialog.getContentElement( 'info', 'charPreview' ).getElement().setHtml( '&nbsp;' );
dialog.getContentElement( 'info', 'htmlPreview' ).getElement().setHtml( '&nbsp;' );
target.getParent().removeClass( 'cke_light_background' );
if ( target.getName() == 'a' ) {
dialog.getContentElement( 'info', 'charPreview' ).getElement().setHtml( '&nbsp;' );
dialog.getContentElement( 'info', 'htmlPreview' ).getElement().setHtml( '&nbsp;' );
target.getParent().removeClass( 'cke_light_background' );

focusedNode = undefined;
}
};
focusedNode = undefined;
}
};

var onKeydown = CKEDITOR.tools.addFunction( function( ev ) {
onKeydown = CKEDITOR.tools.addFunction( function( ev ) {
ev = new CKEDITOR.dom.event( ev );

// Get an Anchor element.
Expand Down Expand Up @@ -214,12 +219,10 @@ CKEDITOR.dialog.add( 'specialchar', function( editor ) {
'</span>' +
'<span class="cke_voice_label" id="' + charLabelId + '">' +
charDesc +
'</span></a>' );
} else {
html.push( '<td class="cke_dark_background">&nbsp;' );
'</span></a></td>'
);
}

html.push( '</td>' );
}
html.push( '</tr>' );
}
Expand Down
34 changes: 34 additions & 0 deletions tests/plugins/specialchar/dialog.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/* bender-tags: specialchar */
/* bender-ckeditor-plugins: toolbar,wysiwygarea,specialchar */

( function() {
'use strict';

bender.editor = true;

bender.test( {
tearDown: function() {
var dialog;

while ( ( dialog = CKEDITOR.dialog.getCurrent() ) ) {
dialog.hide();
}
},

'test specialchar dialog should have table cells with role="presentation" and don\'t have empty table cells': function() {
var bot = this.editorBot;

bot.dialog( 'specialchar', function( dialog ) {
var tableWithCharacters = dialog.parts.contents.findOne( 'td.cke_dialog_ui_hbox_first table[role="listbox"]' ),
tableCells = tableWithCharacters.find( 'td' ).toArray(),
i;

for ( i = 0; i < tableCells.length; i++ ) {
assert.areSame( 'presentation', tableCells[ i ].getAttribute( 'role' ),
'Table cell with index: ' + i + ' should have role="presentation". Instead it has following html: ' + tableCells[ i ].getOuterHtml() );
assert.areNotSame( '&nbsp;', tableCells[ i ].getHtml(), 'Table cell with index: ' + i + ' should not be empty.' );
}
} );
}
} );
} )();
11 changes: 11 additions & 0 deletions tests/plugins/specialchar/manual/presentationrole.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<h2>Classic editor</h2>
<div id="classic">
<p>I'm CKEditor 4 instance.</p>
</div>

<script>
if ( bender.tools.env.mobile ) {
bender.ignore();
}
CKEDITOR.replace( 'classic' );
</script>
20 changes: 20 additions & 0 deletions tests/plugins/specialchar/manual/presentationrole.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
@bender-ui: collapsed
@bender-tags: 3544, 4.13.1, bug
@bender-ckeditor-plugins: wysiwygarea, toolbar, specialchar

1. Open specialchar dialog.
2. Turn on Voice Over on MacOS (`CMD + F5`) or Narrator on Windows (`WIN + CTRL + Enter`).
3. Move selection to the last character.
4. For MacOS: press `CapsLock + Right Arrow`.
5. For Windows: press `Right Arrow`.

In case that you have problem with performing steps with assistive technology:
1. Open console.
2. Inspect if there exists empty table cells after last character in the dialog.

### Expected result:
Assisitve technology doesn't detect and read content of empty table cells at the end of special char dialog.
There shouldn't be any empty cells at the end.

### Unexpected result
Empty table cells are added at the end of characters list.

0 comments on commit 42ff73a

Please sign in to comment.