Skip to content

Commit

Permalink
Merge pull request #7599 from ckeditor/i/7542
Browse files Browse the repository at this point in the history
Fix (widget): Triple-clicking inside an image caption should not crash the editor in Firefox. Closes #7542.

Fix (widget): Triple-clicking a link inside an image caption should not crash the editor in Safari. Closes #6021.
  • Loading branch information
oleq committed Jul 10, 2020
2 parents 2f1568d + 96aa2e7 commit ef4b1f9
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 5 deletions.
1 change: 1 addition & 0 deletions packages/ckeditor5-widget/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"@ckeditor/ckeditor5-heading": "^20.0.0",
"@ckeditor/ckeditor5-horizontal-line": "^20.0.0",
"@ckeditor/ckeditor5-image": "^20.0.0",
"@ckeditor/ckeditor5-link": "^20.0.0",
"@ckeditor/ckeditor5-media-embed": "^20.0.0",
"@ckeditor/ckeditor5-paragraph": "^20.0.0",
"@ckeditor/ckeditor5-table": "^20.0.0",
Expand Down
9 changes: 6 additions & 3 deletions packages/ckeditor5-widget/src/widget.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,12 +146,15 @@ export default class Widget extends Plugin {
// But at least triple click inside nested editable causes broken selection in Safari.
// For such event, we select the entire nested editable element.
// See: https://github.com/ckeditor/ckeditor5/issues/1463.
if ( env.isSafari && domEventData.domEvent.detail >= 3 ) {
if ( ( env.isSafari || env.isGecko ) && domEventData.domEvent.detail >= 3 ) {
const mapper = editor.editing.mapper;
const modelElement = mapper.toModelElement( element );
const viewElement = element.is( 'attributeElement' ) ?
element.findAncestor( element => !element.is( 'attributeElement' ) ) : element;
const modelElement = mapper.toModelElement( viewElement );

domEventData.preventDefault();

this.editor.model.change( writer => {
domEventData.preventDefault();
writer.setSelection( modelElement, 'in' );
} );
}
Expand Down
79 changes: 77 additions & 2 deletions packages/ckeditor5-widget/tests/widget-integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
import Typing from '@ckeditor/ckeditor5-typing/src/typing';
import LinkEditing from '@ckeditor/ckeditor5-link/src/linkediting';
import Widget from '../src/widget';
import DomEventData from '@ckeditor/ckeditor5-engine/src/view/observer/domeventdata';

Expand All @@ -32,7 +33,7 @@ describe( 'Widget - integration', () => {
editorElement = document.createElement( 'div' );
document.body.appendChild( editorElement );

return ClassicEditor.create( editorElement, { plugins: [ Paragraph, Widget, Typing ] } )
return ClassicEditor.create( editorElement, { plugins: [ Paragraph, Widget, Typing, LinkEditing ] } )
.then( newEditor => {
editor = newEditor;
model = editor.model;
Expand All @@ -58,6 +59,9 @@ describe( 'Widget - integration', () => {
isObject: true,
isInline: true
} );
model.schema.extend( '$block', {
allowIn: 'nested'
} );

editor.conversion.for( 'downcast' )
.elementToElement( { model: 'inline', view: 'figure' } )
Expand Down Expand Up @@ -148,6 +152,76 @@ describe( 'Widget - integration', () => {
expect( getModelData( model ) ).to.equal( '<widget><nested>[foo bar]</nested></widget>' );
} );

it( 'should select the entire nested editable if triple clicked on link', () => {
setModelData( model, '[]<widget><nested>foo <$text linkHref="abc">bar</$text></nested></widget>' );

const viewDiv = viewDocument.getRoot().getChild( 0 );
const viewLink = viewDiv.getChild( 0 ).getChild( 1 );
const preventDefault = sinon.spy();
const domEventDataMock = new DomEventData( view, {
target: view.domConverter.mapViewToDom( viewLink ),
preventDefault,
detail: 3
} );

viewDocument.fire( 'mousedown', domEventDataMock );

sinon.assert.called( preventDefault );

expect( getViewData( view ) ).to.equal(
'<div class="ck-widget" contenteditable="false">' +
'<figcaption contenteditable="true">{foo <a href="abc">bar</a>]</figcaption>' +
'<div class="ck ck-reset_all ck-widget__type-around"></div>' +
'</div>'
);
expect( getModelData( model ) ).to.equal( '<widget><nested>[foo <$text linkHref="abc">bar</$text>]</nested></widget>' );
} );

it( 'should select only clicked paragraph if triple clicked on link', () => {
setModelData( model,
'[]<widget>' +
'<nested>' +
'<paragraph>foo</paragraph>' +
'<paragraph>foo <$text linkHref="abc">bar</$text></paragraph>' +
'<paragraph>bar</paragraph>' +
'</nested>' +
'</widget>'
);

const viewDiv = viewDocument.getRoot().getChild( 0 );
const viewLink = viewDiv.getChild( 0 ).getChild( 1 ).getChild( 1 );
const preventDefault = sinon.spy();
const domEventDataMock = new DomEventData( view, {
target: view.domConverter.mapViewToDom( viewLink ),
preventDefault,
detail: 3
} );

viewDocument.fire( 'mousedown', domEventDataMock );

sinon.assert.called( preventDefault );

expect( getViewData( view ) ).to.equal(
'<div class="ck-widget" contenteditable="false">' +
'<figcaption contenteditable="true">' +
'<p>foo</p>' +
'<p>{foo <a href="abc">bar</a>]</p>' +
'<p>bar</p>' +
'</figcaption>' +
'<div class="ck ck-reset_all ck-widget__type-around"></div>' +
'</div>'
);
expect( getModelData( model ) ).to.equal(
'<widget>' +
'<nested>' +
'<paragraph>foo</paragraph>' +
'<paragraph>[foo <$text linkHref="abc">bar</$text>]</paragraph>' +
'<paragraph>bar</paragraph>' +
'</nested>' +
'</widget>'
);
} );

it( 'should select proper nested editable if triple clicked', () => {
setModelData( model, '[]<widget><nested>foo</nested><nested>bar</nested></widget>' );

Expand Down Expand Up @@ -224,8 +298,9 @@ describe( 'Widget - integration', () => {
expect( getModelData( model ) ).to.equal( '<paragraph>Foo[<inline-widget>foo bar</inline-widget>]Bar</paragraph>' );
} );

it( 'should do nothing for non-Safari browser', () => {
it( 'should do nothing for non-Safari and non-Gecko browser', () => {
testUtils.sinon.stub( env, 'isSafari' ).get( () => false );
testUtils.sinon.stub( env, 'isGecko' ).get( () => false );

setModelData( model, '<paragraph>[]</paragraph><widget><nested>foo bar</nested></widget>' );

Expand Down

0 comments on commit ef4b1f9

Please sign in to comment.