Improve selection with widgetselection plugin when widget is partially selected
#2866
Conversation
engineering-this
force-pushed
the
t/2517
branch
from
4e90a9c
to
79a66e6
Compare
There was a problem hiding this comment.
Selection becomes corrupted after the fix.
- Insert new image using procedure in manual test.
- Try to select all using Cmd + A.
Expected:
Editor's content becomes selected.
Actual:
Editor's content becomes unselected after releasing keys.
What's interesting, after switching to source mode and back to WYSIWYG one selection works correctly.
| @@ -0,0 +1,18 @@ | |||
| @bender-tags: 4.11.4, bug | |||
|
Fixed above by further improvements to selection. |
engineering-this
changed the title
|
Updated onto latest |
There was a problem hiding this comment.
Fix is not working in Firefox:
19:51:23.825 TypeError: startNode is null[Więcej informacji] range.js:1920:4
setStart http://tests.ckeditor.test:1030/apps/ckeditor/core/dom/range.js:1920
setStartBefore http://tests.ckeditor.test:1030/apps/ckeditor/core/dom/range.js:1988
fake http://tests.ckeditor.test:1030/apps/ckeditor/core/selection.js:2261
focus http://tests.ckeditor.test:1030/apps/ckeditor/plugins/widget/plugin.js:1426
finalizeCreation http://tests.ckeditor.test:1030/apps/ckeditor/plugins/widget/plugin.js:451
finalizeCreation http://tests.ckeditor.test:1030/apps/ckeditor/plugins/widget/plugin.js:2059
1 http://tests.ckeditor.test:1030/apps/ckeditor/core/event.js:202
listenerFirer http://tests.ckeditor.test:1030/apps/ckeditor/core/event.js:144
fire http://tests.ckeditor.test:1030/apps/ckeditor/core/event.js:290
onClick http://tests.ckeditor.test:1030/apps/ckeditor/plugins/dialog/plugin.js:1621
onClick http://tests.ckeditor.test:1030/apps/ckeditor/plugins/dialogui/plugin.js:964
listenerFirer http://tests.ckeditor.test:1030/apps/ckeditor/core/event.js:144
fire http://tests.ckeditor.test:1030/apps/ckeditor/core/event.js:290
click http://tests.ckeditor.test:1030/apps/ckeditor/plugins/dialogui/plugin.js:916
setTimeout http://tests.ckeditor.test:1030/apps/ckeditor/core/tools.js:578
There was a problem hiding this comment.
I'm not sure that the current behaviour is valid 🤔. In Firefox the widget is visibly also selected, but after inserting widget, it appears after the existing one – yet it should have overwritten it.
OTOH behaviour in Chrome seems to be correct, as there widget isn't selected. In Safari inserting widget into such selection isn't even possible… See #3008.
See also #3007 for very similar issue, but with delete/typing.
plugins/widget/plugin.js
Outdated
|
|
||
| var widget = this.getByElement( wrapper ); | ||
| // Fire postponed #ready event. | ||
| widget.ready = true; | ||
| widget.fire( 'ready' ); | ||
| widget.focus(); | ||
| } | ||
|
|
||
| /** |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
plugins/widget/plugin.js
Outdated
| * @returns {CKEDITOR.dom.node/null} Closest non editable ancestor which is direct child of editable element otherwise `null`. | ||
| */ | ||
| function findOuterNonEditableAncestor( node ) { | ||
| var parents = node.getParents().reverse(), |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Rebased on |
What is the purpose of this pull request?
Bug fix
Does your PR contain necessary tests?
All patches which change the editor code must include tests. You can always read more
on PR testing,
how to set the testing environment and
how to create tests
in the official CKEditor documentation.
This PR contains
What changes did you make?
I've added logic to fix selection when widget is partially selected in
widgetselectionplugin.However when selection is changed when user is creating one with mouse down some issues occur in Safari, so
selectionChangelistener is used only for other ways of selecting than mouse. When selection is made by mouse it will be fixed once button is released.Additionally there was an an error thrown by
range.deleteContentswhich was visible with IE11 selection. Improved if check to prevent error.Closes #2517
Closes #3007
Closes #3008
Closes #3041