Skip to content

Commit

Permalink
Merge pull request #16892 from ckeditor/ck/16806
Browse files Browse the repository at this point in the history
Fix (editor-multi-root): No longer lose selection on clicking editable containing only one block element. Closes #16806
  • Loading branch information
Mati365 authored Aug 27, 2024
2 parents dde1b50 + 0e9db45 commit 06f8520
Show file tree
Hide file tree
Showing 10 changed files with 470 additions and 51 deletions.
30 changes: 4 additions & 26 deletions packages/ckeditor5-clipboard/src/clipboardobserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* @module clipboard/clipboardobserver
*/

import { EventInfo } from '@ckeditor/ckeditor5-utils';
import { EventInfo, getRangeFromMouseEvent } from '@ckeditor/ckeditor5-utils';

import {
DataTransfer,
Expand Down Expand Up @@ -92,7 +92,9 @@ export default class ClipboardObserver extends DomEventObserver<
};

if ( domEvent.type == 'drop' || domEvent.type == 'dragover' ) {
evtData.dropRange = getDropViewRange( this.view, domEvent as DragEvent );
const domRange = getRangeFromMouseEvent( domEvent as DragEvent );

evtData.dropRange = domRange && this.view.domConverter.domRangeToView( domRange );
}

this.fire( domEvent.type, domEvent, evtData );
Expand All @@ -115,30 +117,6 @@ export interface ClipboardEventData {
dropRange?: ViewRange | null;
}

function getDropViewRange( view: EditingView, domEvent: DragEvent & { rangeParent?: Node; rangeOffset?: number } ) {
const domDoc = ( domEvent.target as Node ).ownerDocument!;
const x = domEvent.clientX;
const y = domEvent.clientY;
let domRange;

// Webkit & Blink.
if ( domDoc.caretRangeFromPoint && domDoc.caretRangeFromPoint( x, y ) ) {
domRange = domDoc.caretRangeFromPoint( x, y );
}
// FF.
else if ( domEvent.rangeParent ) {
domRange = domDoc.createRange();
domRange.setStart( domEvent.rangeParent, domEvent.rangeOffset! );
domRange.collapse( true );
}

if ( domRange ) {
return view.domConverter.domRangeToView( domRange );
}

return null;
}

/**
* Fired as a continuation of the {@link module:engine/view/document~Document#event:paste} and
* {@link module:engine/view/document~Document#event:drop} events.
Expand Down
3 changes: 2 additions & 1 deletion packages/ckeditor5-clipboard/tests/dragdrop.js
Original file line number Diff line number Diff line change
Expand Up @@ -2436,7 +2436,8 @@ describe( 'Drag and Drop', () => {
const eventData = prepareEventData( model.document.selection.getLastPosition(), domTarget );

viewDocument.fire( 'mousedown', {
...eventData
...eventData,
preventDefault
} );

viewDocument.fire( 'dragstart', {
Expand Down
5 changes: 4 additions & 1 deletion packages/ckeditor5-editor-multi-root/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
"devDependencies": {
"@ckeditor/ckeditor5-basic-styles": "43.0.0",
"@ckeditor/ckeditor5-dev-utils": "^42.0.0",
"@ckeditor/ckeditor5-essentials": "43.0.0",
"@ckeditor/ckeditor5-enter": "43.0.0",
"@ckeditor/ckeditor5-heading": "43.0.0",
"@ckeditor/ckeditor5-paragraph": "43.0.0",
Expand All @@ -32,6 +31,10 @@
"@ckeditor/ckeditor5-undo": "43.0.0",
"@ckeditor/ckeditor5-clipboard": "43.0.0",
"@ckeditor/ckeditor5-watchdog": "43.0.0",
"@ckeditor/ckeditor5-image": "43.0.0",
"@ckeditor/ckeditor5-link": "43.0.0",
"@ckeditor/ckeditor5-adapter-ckfinder": "43.0.0",
"@ckeditor/ckeditor5-ckfinder": "43.0.0",
"typescript": "5.0.4",
"webpack": "^5.58.1",
"webpack-cli": "^4.9.0"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
<head>
<meta http-equiv="Content-Security-Policy" content="script-src 'self' https://ckeditor.com 'unsafe-inline' 'unsafe-eval'">
</head>

<script src="https://ckeditor.com/apps/ckfinder/3.5.0/ckfinder.js"></script>

<p>
<button id="destroyEditor">Destroy editor</button>
<button id="initEditor">Init editor</button>
Expand All @@ -12,7 +18,19 @@ <h2>The toolbar</h2>
<h2>The editable</h2>
<div class="editable-container">
<div id="editor-intro"><p><strong>Exciting</strong> intro text to an article.</p></div>
<div id="editor-content"><h2>Exciting news!</h2><p>Lorem ipsum dolor sit amet.</p></div>
<div id="editor-content">
<table>
<tr>
<td>First cell</td>
<td>Second cell</td>
</tr>
</table>

<br />
<br />

<p>Hello World</p>
</div>
<div id="editor-outro"><p>Closing text.</p></div>
</div>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,13 @@ import Heading from '@ckeditor/ckeditor5-heading/src/heading.js';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph.js';
import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold.js';
import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic.js';
import Essentials from '@ckeditor/ckeditor5-essentials/src/essentials.js';
import Image from '@ckeditor/ckeditor5-image/src/image.js';
import AutoImage from '@ckeditor/ckeditor5-image/src/autoimage.js';
import ImageInsert from '@ckeditor/ckeditor5-image/src/imageinsert.js';
import LinkImage from '@ckeditor/ckeditor5-link/src/linkimage.js';
import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset.js';
import CKFinderUploadAdapter from '@ckeditor/ckeditor5-adapter-ckfinder/src/uploadadapter.js';
import CKFinder from '@ckeditor/ckeditor5-ckfinder/src/ckfinder.js';

const editorData = {
intro: document.querySelector( '#editor-intro' ),
Expand All @@ -23,8 +29,26 @@ let editor;
function initEditor() {
MultiRootEditor
.create( editorData, {
plugins: [ Essentials, Paragraph, Heading, Bold, Italic ],
toolbar: [ 'heading', '|', 'bold', 'italic', 'undo', 'redo' ]
plugins: [
Paragraph, Heading, Bold, Italic,
Image, ImageInsert, AutoImage, LinkImage,
ArticlePluginSet, CKFinderUploadAdapter, CKFinder
],
toolbar: [
'heading', '|', 'bold', 'italic', 'undo', 'redo', '|',
'insertImage', 'insertTable', 'blockQuote'
],
image: {
toolbar: [
'imageStyle:inline', 'imageStyle:block',
'imageStyle:wrapText', '|', 'toggleImageCaption',
'imageTextAlternative'
]
},
ckfinder: {
// eslint-disable-next-line max-len
uploadUrl: 'https://ckeditor.com/apps/ckfinder/3.5.0/core/connector/php/connector.php?command=QuickUpload&type=Files&responseType=json'
}
} )
.then( newEditor => {
console.log( 'Editor was initialized', newEditor );
Expand Down Expand Up @@ -55,3 +79,5 @@ function destroyEditor() {

document.getElementById( 'initEditor' ).addEventListener( 'click', initEditor );
document.getElementById( 'destroyEditor' ).addEventListener( 'click', destroyEditor );

initEditor();
44 changes: 44 additions & 0 deletions packages/ckeditor5-utils/src/dom/getrangefrommouseevent.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/**
* @license Copyright (c) 2003-2024, CKSource Holding sp. z o.o. All rights reserved.
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

/**
* @module utils/dom/getrangefrommouseevent
*/

/**
* Returns a DOM range from a given point specified by a mouse event.
*
* @param domEvent The mouse event.
* @returns The DOM range.
*/
export default function getRangeFromMouseEvent(
domEvent: MouseEvent & {
rangeParent?: HTMLElement;
rangeOffset?: number;
}
): Range | null {
if ( !domEvent.target ) {
return null;
}

const domDoc = ( domEvent.target as HTMLElement ).ownerDocument;
const x = domEvent.clientX;
const y = domEvent.clientY;
let domRange = null;

// Webkit & Blink.
if ( domDoc.caretRangeFromPoint && domDoc.caretRangeFromPoint( x, y ) ) {
domRange = domDoc.caretRangeFromPoint( x, y );
}

// FF.
else if ( domEvent.rangeParent ) {
domRange = domDoc.createRange();
domRange.setStart( domEvent.rangeParent, domEvent.rangeOffset! );
domRange.collapse( true );
}

return domRange;
}
1 change: 1 addition & 0 deletions packages/ckeditor5-utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export { default as global } from './dom/global.js';
export { default as getAncestors } from './dom/getancestors.js';
export { default as getDataFromElement } from './dom/getdatafromelement.js';
export { default as getBorderWidths } from './dom/getborderwidths.js';
export { default as getRangeFromMouseEvent } from './dom/getrangefrommouseevent.js';
export { default as isText } from './dom/istext.js';
export { default as Rect, type RectSource } from './dom/rect.js';
export { default as ResizeObserver } from './dom/resizeobserver.js';
Expand Down
76 changes: 76 additions & 0 deletions packages/ckeditor5-utils/tests/dom/getrangefrommouseevent.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/**
* @license Copyright (c) 2003-2024, CKSource Holding sp. z o.o. All rights reserved.
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

import getRangeFromMouseEvent from '../../src/dom/getrangefrommouseevent.js';

describe( 'getRangeFromMouseEvent()', () => {
it( 'should use Document#caretRangeFromPoint method to obtain range on Webkit & Blink', () => {
const fakeRange = {
startOffset: 0,
endOffset: 0
};

const caretRangeFromPointSpy = sinon.stub().returns( fakeRange );
const evt = {
clientX: 10,
clientY: 11,
target: {
ownerDocument: {
caretRangeFromPoint: caretRangeFromPointSpy
}
}
};

expect( getRangeFromMouseEvent( evt ) ).to.be.equal( fakeRange );
expect( caretRangeFromPointSpy ).to.be.calledWith( 10, 11 );
} );

it( 'should use Document#createRange method to obtain range on Firefox', () => {
const fakeRange = {
startOffset: 0,
endOffset: 0,
setStart: sinon.stub(),
collapse: sinon.stub()
};

const evt = {
clientX: 10,
clientY: 11,
rangeOffset: 13,
rangeParent: { parent: true },
target: {
ownerDocument: {
createRange: sinon.stub().returns( fakeRange )
}
}
};

expect( getRangeFromMouseEvent( evt ) ).to.be.equal( fakeRange );

expect( fakeRange.collapse ).to.be.calledWith( true );
expect( fakeRange.setStart ).to.be.calledWith( evt.rangeParent, evt.rangeOffset );
} );

it( 'should return null if event target is null', () => {
const evt = {
target: null
};

expect( getRangeFromMouseEvent( evt ) ).to.be.null;
} );

it( 'should return null if event target is not null but it\'s not possible to create range on document', () => {
const evt = {
target: {
ownerDocument: {
createRange: null,
caretRangeFromPoint: null
}
}
};

expect( getRangeFromMouseEvent( evt ) ).to.be.null;
} );
} );
Loading

0 comments on commit 06f8520

Please sign in to comment.