Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix type error while opening context menu in readonly editor #2331

Merged
merged 10 commits into from
Sep 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@

## CKEditor 4.10.2

Fixed Issues:

* [#1181](https://github.com/ckeditor/ckeditor-dev/issues/1181): [Chrome] Fixed: Opening context menu in readonly editor results in error.

## CKEditor 4.10.1

Fixed Issues:
Expand Down
4 changes: 3 additions & 1 deletion plugins/contextmenu/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,9 @@ CKEDITOR.plugins.add( 'contextmenu', {
* @param {Number} [offsetY]
*/
open: function( offsetParent, corner, offsetX, offsetY ) {
if ( this.editor.config.enableContextMenu === false ) {
// Do not open context menu if it's disabled or there is no selection in the editor (#1181).
if ( this.editor.config.enableContextMenu === false ||
this.editor.getSelection().getType() === CKEDITOR.SELECTION_NONE ) {
return;
}

Expand Down
15 changes: 13 additions & 2 deletions tests/_benderjs/ckeditor/static/bot.js
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,8 @@
*/
contextmenu: function( callback ) {
var editor = this.editor,
tc = this.testCase;
tc = this.testCase,
range;

editor.once( 'panelShow', function() {
// Make sure resume comes after wait.
Expand All @@ -245,10 +246,20 @@
} );
} );

// Force selection in the editor as opening menu
// by user always results in selection in non readonly editor.
if ( !editor.readOnly && editor.getSelection().getType() === CKEDITOR.SELECTION_NONE ) {
range = editor.createRange();

range.selectNodeContents( editor.editable() );
range.collapse( true );
range.select();
}

// Open context menu on editable element.
editor.contextMenu.open( editor.editable() );

// combo panel opening is synchronous;
// Combo panel opening is asynchronous.
tc.wait();
},

Expand Down
21 changes: 19 additions & 2 deletions tests/plugins/contextmenu/contextmenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
var ed1 = this.editor,
bot1 = this.editorBot;

ed1.focus();
bot1.contextmenu( function( menu1 ) {
// Check DOM focus and virtual editor focus.
assert.areSame( menu1._.panel._.iframe, CKEDITOR.document.getActive(), 'check DOM focus inside of panel' );
Expand All @@ -36,7 +37,7 @@
name: 'editor_nocontextmenu1'
}, function( bot ) {
bot.editor.contextMenu.show = sinon.spy();

bot.editor.focus();
bot.editor.contextMenu.open( bot.editor.editable() );

assert.isTrue( bot.editor.contextMenu.show.called );
Expand All @@ -51,7 +52,7 @@
}
}, function( bot ) {
bot.editor.contextMenu.show = sinon.spy();

bot.editor.focus();
bot.editor.contextMenu.open( bot.editor.editable() );

assert.isFalse( bot.editor.contextMenu.show.called );
Expand Down Expand Up @@ -121,6 +122,22 @@
assert.isFalse( !!editor.getSelection().isFake, 'Fake selection is not set for nested editables' );
} );
} );
},

// (#1181)
'test open context menu when no selection in the editor': function() {
bender.editorBot.create( {
name: 'editor_noselection'
}, function( bot ) {
var editor = bot.editor;

editor.contextMenu.show = sinon.spy();

editor.getSelection().removeAllRanges();
editor.contextMenu.open( editor.editable() );

assert.isFalse( editor.contextMenu.show.called );
} );
}
} );
} )();
4 changes: 4 additions & 0 deletions tests/plugins/contextmenu/integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
} );
} );

editor.focus();
editor.contextMenu.open( editor.editable() );

wait();
Expand Down Expand Up @@ -55,6 +56,7 @@
} );
} );

editor.focus();
editor.contextMenu.open( editor.editable() );

wait();
Expand Down Expand Up @@ -87,6 +89,7 @@
editor.contextMenu.open( editor.editable() );
} );

editor.focus();
editor.contextMenu.open( editor.editable() );

wait();
Expand All @@ -103,6 +106,7 @@
} );
} );

bot.editor.focus();
bot.editor.contextMenu.open( bot.editor.editable() );

wait();
Expand Down
13 changes: 13 additions & 0 deletions tests/plugins/contextmenu/manual/readonly.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<div id="editor">
<p>I am a sample content with <a href="https://ckeditor.com">link</a>.</p>
</div>

<script>
if ( CKEDITOR.env.mobile || !CKEDITOR.env.chrome ) {
bender.ignore();
}

CKEDITOR.replace( 'editor', {
readOnly: true
} );
</script>
20 changes: 20 additions & 0 deletions tests/plugins/contextmenu/manual/readonly.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
@bender-ui: collapsed
@bender-tags: bug, 4.10.2, 1181
@bender-ckeditor-plugins: wysiwygarea, toolbar, contextmenu, link, clipboard

1. Open console.
2. Right click on the link without prior focusing the editor.

## Expected

Context menu does not appear.

In case of Chrome on macOS: right-clicking on link will select it and open the context menu.

## Unexpected

There is an error inside the console:

```
Uncaught TypeError: Cannot read property 'checkReadOnly' of undefined
```