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

Commit

Permalink
Fix: Complex selection brakes when spanning over widget.
Browse files Browse the repository at this point in the history
  • Loading branch information
jodator committed Jul 5, 2018
1 parent f5bffd4 commit 2445ef2
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 44 deletions.
52 changes: 11 additions & 41 deletions src/model/utils/selection-post-fixer.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,18 @@ function selectionPostFixer( writer, model ) {
if ( wasFixed ) {
// The above algorithm might create ranges that intersects each other when selection contains more then one range.
// This is case happens mostly on Firefox which creates multiple ranges for selected table.
const combinedRanges = combineOverlappingRanges( ranges );
let fixedRanges = ranges;

writer.setSelection( combinedRanges, { backward: selection.isBackward } );
// Fixing selection with many ranges usually breaks the selection in Firefox. As only Firefox supports multiple selection ranges
// we simply create one continuous range from fixed selection ranges (even if they are not adjacent).
if ( ranges.length > 1 ) {
const selectionStart = ranges[ 0 ].start;
const selectionEnd = ranges[ ranges.length - 1 ].end;

fixedRanges = [ new Range( selectionStart, selectionEnd ) ];
}

writer.setSelection( fixedRanges, { backward: selection.isBackward } );
}
}

Expand Down Expand Up @@ -220,45 +229,6 @@ function expandSelectionOnIsLimitNode( position, schema, expandToDirection ) {
return expandToDirection === 'start' ? Position.createBefore( node ) : Position.createAfter( node );
}

// Returns minimal set of continuous ranges.
//
// @param {Array.<module:engine/model/range~Range>} ranges
// @returns {Array.<module:engine/model/range~Range>}
function combineOverlappingRanges( ranges ) {
const combinedRanges = [];

// Seed the state.
let previousRange = ranges[ 0 ];
combinedRanges.push( previousRange );

// Go through each ranges and check if it can be merged with previous one.
for ( const range of ranges ) {
// Do not push same ranges (ie might be created in a table).
if ( range.isEqual( previousRange ) ) {
continue;
}

// Merge intersecting range into previous one.
if ( range.isIntersecting( previousRange ) ) {
const newStart = previousRange.start.isBefore( range.start ) ? previousRange.start : range.start;
const newEnd = range.end.isAfter( previousRange.end ) ? range.end : previousRange.end;
const combinedRange = new Range( newStart, newEnd );

// Replace previous range with the combined one.
combinedRanges.splice( combinedRanges.indexOf( previousRange ), 1, combinedRange );

previousRange = combinedRange;

continue;
}

previousRange = range;
combinedRanges.push( range );
}

return combinedRanges;
}

// Checks whether both range ends are placed around non-limit elements.
//
// @param {module:engine/model/position~Position} start
Expand Down
6 changes: 3 additions & 3 deletions tests/model/utils/selection-post-fixer.js
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ describe( 'Selection post-fixer', () => {
'<table>' +
'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
'</table>' +
'<paragraph>b]a[r]</paragraph>'
'<paragraph>bar]</paragraph>'
);
} );
} );
Expand Down Expand Up @@ -746,7 +746,7 @@ describe( 'Selection post-fixer', () => {
} );

it( 'should fix multiple ranges #1', () => {
// []<paragraph></paragraph>[]<table>...
// []<paragraph>foo</paragraph>[]<table>...
model.change( writer => {
writer.setSelection(
[
Expand All @@ -757,7 +757,7 @@ describe( 'Selection post-fixer', () => {
} );

expect( getModelData( model ) ).to.equal(
'<paragraph>[]foo[]</paragraph>' +
'<paragraph>[foo]</paragraph>' +
'<table>' +
'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
'</table>' +
Expand Down

0 comments on commit 2445ef2

Please sign in to comment.