Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

I/5840: Safe fixes to enforcing restricted editing restrictions #7

Merged
merged 14 commits into from
Dec 19, 2019
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
22 changes: 11 additions & 11 deletions src/restrictededitingmode/converters.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,9 @@ export function extendMarkerOnTypingPostFixer( editor ) {
let changeApplied = false;

for ( const change of editor.model.document.differ.getChanges() ) {
if ( change.type == 'insert' && change.name == '$text' && change.length === 1 ) {
changeApplied = _tryExtendMarkerStart( editor, change.position, writer ) || changeApplied;
changeApplied = _tryExtendMarkedEnd( editor, change.position, writer ) || changeApplied;
if ( change.type == 'insert' && change.name == '$text' ) {
changeApplied = _tryExtendMarkerStart( editor, change.position, change.length, writer ) || changeApplied;
changeApplied = _tryExtendMarkedEnd( editor, change.position, change.length, writer ) || changeApplied;
}
}

Expand Down Expand Up @@ -159,13 +159,13 @@ export function upcastHighlightToMarker( config ) {
} );
}

// Extend marker if typing detected on marker's start position.
function _tryExtendMarkerStart( editor, position, writer ) {
const markerAtStart = getMarkerAtPosition( editor, position.getShiftedBy( 1 ) );
// Extend marker if change detected on marker's start position.
function _tryExtendMarkerStart( editor, position, length, writer ) {
const markerAtStart = getMarkerAtPosition( editor, position.getShiftedBy( length ) );

if ( markerAtStart && markerAtStart.getStart().isEqual( position.getShiftedBy( 1 ) ) ) {
if ( markerAtStart && markerAtStart.getStart().isEqual( position.getShiftedBy( length ) ) ) {
writer.updateMarker( markerAtStart, {
range: writer.createRange( markerAtStart.getStart().getShiftedBy( -1 ), markerAtStart.getEnd() )
range: writer.createRange( markerAtStart.getStart().getShiftedBy( -length ), markerAtStart.getEnd() )
} );

return true;
Expand All @@ -174,13 +174,13 @@ function _tryExtendMarkerStart( editor, position, writer ) {
return false;
}

// Extend marker if typing detected on marker's end position.
function _tryExtendMarkedEnd( editor, position, writer ) {
// Extend marker if change detected on marker's end position.
function _tryExtendMarkedEnd( editor, position, length, writer ) {
const markerAtEnd = getMarkerAtPosition( editor, position );

if ( markerAtEnd && markerAtEnd.getEnd().isEqual( position ) ) {
writer.updateMarker( markerAtEnd, {
range: writer.createRange( markerAtEnd.getStart(), markerAtEnd.getEnd().getShiftedBy( 1 ) )
range: writer.createRange( markerAtEnd.getStart(), markerAtEnd.getEnd().getShiftedBy( length ) )
} );

return true;
Expand Down
87 changes: 87 additions & 0 deletions src/restrictededitingmodeediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ export default class RestrictedEditingModeEditing extends Plugin {

this._setupConversion();
this._setupCommandsToggling();
this._setupRestrictions();

// Commands & keystrokes that allow navigation in the content.
editor.commands.add( 'goToPreviousRestrictedEditingException', new RestrictedEditingNavigationCommand( editor, 'backward' ) );
Expand Down Expand Up @@ -165,6 +166,25 @@ export default class RestrictedEditingModeEditing extends Plugin {
setupExceptionHighlighting( editor );
}

/**
* Setups additional editing restrictions beyond command toggling.
*
* @private
*/
_setupRestrictions() {
const editor = this.editor;

this.listenTo( editor.model, 'deleteContent', restrictDeleteContent( editor ), { priority: 'high' } );

const inputCommand = this.editor.commands.get( 'input' );

// The restricted editing might be configured without input support - ie allow only bolding or removing text.
// This check is bit synthetic since only tests are used this way.
if ( inputCommand ) {
this.listenTo( inputCommand, 'execute', disallowInputExecForWrongRange( editor ), { priority: 'high' } );
}
}

/**
* Setups the commands toggling - enables or disables commands based on user selection.
*
Expand Down Expand Up @@ -284,3 +304,70 @@ function filterDeleteCommandsOnMarkerBoundaries( selection, markerRange ) {
return true;
};
}

// Ensures that model.deleteContent() does not delete outside exception markers ranges.
//
// The enforced restrictions are:
// - only execute deleteContent() inside exception markers
// - restrict passed selection to exception marker
function restrictDeleteContent( editor ) {
return ( evt, args ) => {
const [ selection ] = args;

const marker = getMarkerAtPosition( editor, selection.focus ) || getMarkerAtPosition( editor, selection.anchor );

// Stop method execution if marker was not found at selection focus.
if ( !marker ) {
evt.stop();

return;
}

// Collapsed selection inside exception marker does not require fixing.
if ( selection.isCollapsed ) {
return;
}

// Shrink the selection to the range inside exception marker.
const allowedToDelete = marker.getRange().getIntersection( selection.getFirstRange() );

// Some features uses selection passed to model.deleteContent() to set the selection afterwards. For this we need to properly modify
// either the document selection using change block...
if ( selection.is( 'documentSelection' ) ) {
editor.model.change( writer => {
writer.setSelection( allowedToDelete );
} );
}
// ... or by modifying passed selection instance directly.
else {
selection.setTo( allowedToDelete );
}
};
}

// Ensures that input command is executed with a range that is inside exception marker.
//
// This restriction is due to fact that using native spell check changes text outside exception marker.
function disallowInputExecForWrongRange( editor ) {
return ( evt, args ) => {
const [ options ] = args;
const { range } = options;

// Only check "input" command executed with a range value.
// Selection might be set in exception marker but passed range might point elsewhere.
if ( !range ) {
return;
}

if ( !isRangeInsideSingleMarker( editor, range ) ) {
evt.stop();
}
};
}

function isRangeInsideSingleMarker( editor, range ) {
const markerAtStart = getMarkerAtPosition( editor, range.start );
const markerAtEnd = getMarkerAtPosition( editor, range.end );

return markerAtStart && markerAtEnd && markerAtEnd === markerAtStart;
}
1 change: 1 addition & 0 deletions tests/manual/restrictedediting.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
<div id="editor">
<h2>Heading 1</h2>
<p>Paragraph <span class="restricted-editing-exception">it is editable</span></p>
<p>Exception on part of a word: <a href="ckeditor.com">coompi</a><span class="restricted-editing-exception"><a href="ckeditor.com">ter</a> (fix spelling in Firefox).</span></p>
<p><strong>Bold</strong> <i>Italic</i> <a href="foo">Link</a></p>
<ul>
<li>UL List item 1</li>
Expand Down
Loading