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 for: Can't open context menu with keyboard shortcut over widget #2958

Merged
merged 24 commits into from
Apr 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
f5625d9
Allow SHIFT keystrokes to be passed to other listeners alongside ALT …
Dumluregn Mar 14, 2019
be7ba03
Add manual tests.
Dumluregn Mar 14, 2019
10d4576
Change widget in use from Mathjax to Placeholder for simplification.
Dumluregn Mar 15, 2019
9cbe074
Simplify instruction in test scenarios and remove spelling errors.
Dumluregn Mar 15, 2019
4e938de
Split long conditions line.
Dumluregn Mar 15, 2019
3a146dd
Add missing line feed at the end of file.
Dumluregn Mar 15, 2019
f43caf2
Add unit tests.
Dumluregn Mar 15, 2019
70458b0
Move comment to the right place.
Dumluregn Mar 25, 2019
ac49397
Add ticket references.
Dumluregn Mar 25, 2019
3629095
Move tests to appropriate file.
Dumluregn Mar 25, 2019
799fec0
Add tests for displaying context menu.
Dumluregn Mar 25, 2019
c7a1a7a
Update changelog entry.
jacekbogdanski Mar 26, 2019
733acc4
Put ticket references into brackets.
Dumluregn Mar 26, 2019
5585540
Replace variable name with more meaningful one.
Dumluregn Mar 26, 2019
871a5d9
Simplify code using sinon.js library.
Dumluregn Mar 26, 2019
66e415a
Add breaking lines for readability and change assertion method for be…
Dumluregn Mar 26, 2019
8a026ea
Updated unit test to be more generic.
jacekbogdanski Mar 27, 2019
18f9a3a
Small refactoring.
jacekbogdanski Mar 27, 2019
381c445
Small manual test readibility improvement.
jacekbogdanski Mar 27, 2019
4d3a8a1
Updated manual test description.
jacekbogdanski Mar 27, 2019
6f4fe39
Remove unused variable declaration to fix jshint.
Dumluregn Apr 1, 2019
9313845
Enable only SHIFT + F10 hotkey due to problems with other keyboard sh…
Dumluregn Apr 1, 2019
1355f27
Add manual tests for block widget and other keyboard shortcuts.
Dumluregn Apr 1, 2019
c01a48c
Change tests and solution basing on code review.
Dumluregn Apr 1, 2019
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
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Fixed Issues:
* [#2816](https://github.com/ckeditor/ckeditor-dev/issues/2816): Fixed: [Enhanced Image](https://ckeditor.com/cke4/addon/image2) resize handler is visible in [read-only mode](https://ckeditor.com/docs/ckeditor4/latest/guide/dev_readonly.html).
* [#2874](https://github.com/ckeditor/ckeditor-dev/issues/2874): Fixed: [Enhanced Image](https://ckeditor.com/cke4/addon/image2) resize handler is not added when editor is initialized in [read-only mode](https://ckeditor.com/docs/ckeditor4/latest/guide/dev_readonly.html).
* [#2775](https://github.com/ckeditor/ckeditor-dev/issues/2775): Fixed: [Clipboard](https://ckeditor.com/cke4/addon/clipboard) paste buttons have wrong state when [read-only](https://ckeditor.com/docs/ckeditor4/latest/guide/dev_readonly.html) mode is set by mouse event listener with [Div Editing Area](https://ckeditor.com/cke4/addon/divarea) plugin.
* [#1901](https://github.com/ckeditor/ckeditor-dev/issues/1901): Fixed: Can't open a context menu with keyboard shortcut <kbd>Shift</kbd>+<kbd>F10</kbd> over a [Widget](https://ckeditor.com/cke4/addon/widget).

## CKEditor 4.11.3

Expand Down
16 changes: 10 additions & 6 deletions plugins/widget/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -3104,6 +3104,9 @@
// LEFT, RIGHT, UP, DOWN, DEL, BACKSPACE - unblock default fake sel handlers.
var keystrokesNotBlockedByWidget = { 37: 1, 38: 1, 39: 1, 40: 1, 8: 1, 46: 1 };

// Do not block SHIFT + F10 which opens context menu (#1901).
keystrokesNotBlockedByWidget[ CKEDITOR.SHIFT + 121 ] = 1;

// Applies or removes style's classes from widget.
// @param {CKEDITOR.style} style Custom widget style.
// @param {Boolean} apply Whether to apply or remove style.
Expand Down Expand Up @@ -3570,13 +3573,15 @@
// ENTER.
if ( keyCode == 13 ) {
widget.edit();
// CTRL+C or CTRL+X.
// CTRL+C or CTRL+X.
} else if ( keyCode == CKEDITOR.CTRL + 67 || keyCode == CKEDITOR.CTRL + 88 ) {
copySingleWidget( widget, keyCode == CKEDITOR.CTRL + 88 );
return; // Do not preventDefault.
} else if ( keyCode in keystrokesNotBlockedByWidget || ( CKEDITOR.CTRL & keyCode ) || ( CKEDITOR.ALT & keyCode ) ) {
// Pass chosen keystrokes to other plugins or default fake sel handlers.
// Pass all CTRL/ALT keystrokes.
// Pass chosen keystrokes to other plugins or default fake sel handlers.
// Pass all CTRL/ALT keystrokes.
} else if ( keyCode in keystrokesNotBlockedByWidget ||
( CKEDITOR.CTRL & keyCode ) ||
( CKEDITOR.ALT & keyCode ) ) {
return;
}

Expand Down Expand Up @@ -3638,8 +3643,7 @@

function addCustomStyleHandler() {
// Styles categorized by group. It is used to prevent applying styles for the same group being used together.
var styleGroups = {},
styleDefinitions = [];
var styleGroups = {};

/**
* The class representing a widget style. It is an {@link CKEDITOR#STYLE_OBJECT object} like
Expand Down
53 changes: 52 additions & 1 deletion tests/plugins/widget/contextmenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,57 @@
var getWidgetById = widgetTestsTools.getWidgetById;

bender.test( {
// (#1901)
'test SHIFT + F10 shortcut upon widget focus': function() {
Dumluregn marked this conversation as resolved.
Show resolved Hide resolved
var editor = this.editor;

editor.widgets.add( 'testevent', {
editables: {
foo: '.foo'
}
} );

this.editorBot.setData(
'<p id="p1">foo</p><div data-widget="testevent" id="w1"><p class="foo">foo</p></div>',
function() {
var widget = getWidgetById( editor, 'w1' ),
spy = sinon.spy();

widget.on( 'contextMenu', spy );
widget.focus();

editor.editable().fire( 'keydown', new CKEDITOR.dom.event( { keyCode: CKEDITOR.SHIFT + 121 } ) );

assert.isTrue( spy.calledOnce );
}
);
},

'test CTRL + SHIFT + F10 shortcut upon widget focus': function() {
Dumluregn marked this conversation as resolved.
Show resolved Hide resolved
var editor = this.editor;

editor.widgets.add( 'testevent', {
editables: {
foo: '.foo'
}
} );

this.editorBot.setData(
'<p id="p1">foo</p><div data-widget="testevent" id="w1"><p class="foo">foo</p></div>',
function() {
var widget = getWidgetById( editor, 'w1' ),
spy = sinon.spy();

widget.on( 'contextMenu', spy );
widget.focus();
Dumluregn marked this conversation as resolved.
Show resolved Hide resolved

editor.editable().fire( 'keydown', new CKEDITOR.dom.event( { keyCode: CKEDITOR.CTRL + CKEDITOR.SHIFT + 121 } ) );

assert.isTrue( spy.calledOnce );
}
);
},

'test contextMenu event firing': function() {
var editor = this.editor;

Expand Down Expand Up @@ -127,4 +178,4 @@
} );
}
} );
} )();
} )();
15 changes: 15 additions & 0 deletions tests/plugins/widget/manual/contextmenushortcuts.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<div id="editor">
<p>Foo [[Focus me!]] Bar</p>
</div>
<div id="downcast-php">
<p>Foo</p>
<pre>
<code class="language-php">Hello world!</code>
</pre>
<p>Bar</p>
</div>

Dumluregn marked this conversation as resolved.
Show resolved Hide resolved
<script>
CKEDITOR.replace( 'editor' );
CKEDITOR.replace( 'downcast-php' );
</script>
45 changes: 45 additions & 0 deletions tests/plugins/widget/manual/contextmenushortcuts.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
@bender-tags: 4.11.4, bug, 1901, widget, accessibility
@bender-ui: collapsed
@bender-ckeditor-plugins: widget, wysiwygarea, contextmenu, placeholder, codesnippet, enterkey, undo

## Keyboard shortcuts for widgets' context menu

Please do each test for both **inline** (first editor) and **block** (second editor) widget.

Test should be performed with browser dev tools opened.

### Scenario 1: `SHIFT + F10`

1. Focus widget in editor.
1. Press `SHIFT + F10`.

**Expected:** Context menu opens.

### Scenario 2: `CTRL + SHIFT + F10`

1. Focus widget in editor.
1. Press `CTRL + SHIFT + F10`.

**Expected:** Context menu opens.

### Scenario 3: `SHIFT + LEFT`

1. Focus widget in editor.
1. Press `SHIFT + LEFT`.

**Expected:** There should be no error in browser console.

### Scenario 4: `SHIFT + ENTER`

1. Focus widget in editor.
1. Press `SHIFT + ENTER` twice.
1. Click `Undo` button.

**Expected:** New line should not be inserted and browser console should remain clean.

### Scenario 5: `SHIFT + Tab`

1. Focus widget in editor.
1. Press `SHIFT + TAB`.

**Expected:** Widget should still be focused.
24 changes: 24 additions & 0 deletions tests/plugins/widget/widgetsintegration.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,30 @@
this.editor.setReadOnly( false );
},

// (#1901)
'test modifier keystrokes upon widget focus': function() {
var editor = this.editor;

editor.widgets.add( 'testevent', {
editables: {
foo: '.foo'
}
} );

this.editorBot.setData( '<p id="p1">foo</p><div data-widget="testevent" id="w1"><p class="foo">foo</p></div>', function() {
var widget = getWidgetById( editor, 'w1' );

widget.focus();
assert.areNotEqual( false, widget.fire( 'key', { keyCode: CKEDITOR.SHIFT + 121 } ), 'Shift should not be canceled' ); // SHIFT + F10

widget.focus();
assert.areNotEqual( false, widget.fire( 'key', { keyCode: CKEDITOR.CTRL + 121 } ), 'Ctrl should not be canceled' ); // CTRL + F10

widget.focus();
assert.areNotEqual( false, widget.fire( 'key', { keyCode: CKEDITOR.ALT + 121 } ), 'Alt should not be canceled' ); // ALT + F10
} );
},

'test initializing widgets': function() {
var editor = this.editor;

Expand Down