Skip to content

Commit 5ea2a38

Browse files
ReinmarReinmar
committed
Merge branch 't/12178'
2 parents 18c6781 + e02e793 commit 5ea2a38

File tree

4 files changed

+219
-14
lines changed

4 files changed

+219
-14
lines changed

‎CHANGES.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ Fixed Issues:
1414
* [#12243](http://dev.ckeditor.com/ticket/12243): Fixed: Text formatting lost while pasting from Word. Thanks to [Alin Purcaru](https://github.com/mesmerizero)!
1515
* [#12273](http://dev.ckeditor.com/ticket/12273): Fixed: Applying block style in description list breaks it.
1616
* [#12281](http://dev.ckeditor.com/ticket/12281): Fixed: Minor syntax issue in CSS files.
17+
* [#12178](http://dev.ckeditor.com/ticket/12178): [Blink/Webkit] Fixed: Iterator does not return block if selection is located at the end of it.
1718

1819
## CKEditor 4.4.3
1920

‎core/dom/iterator.js

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,11 +361,28 @@
361361
function startIterator() {
362362
var range = this.range.clone(),
363363
// Indicate at least one of the range boundaries is inside a preformat block.
364-
touchPre;
364+
touchPre,
365+
366+
// (#12178)
367+
// Remember if following situation takes place:
368+
// * startAtInnerBoundary: <p>foo[</p>...
369+
// * endAtInnerBoundary: ...<p>]bar</p>
370+
// Because information about line break will be lost when shrinking range.
371+
// Note that we test only if path block exist, because we must properly shrink
372+
// range containing table and/or table cells.
373+
startPath = range.startPath(),
374+
endPath = range.endPath(),
375+
startAtInnerBoundary = rangeAtInnerBlockBoundary( range, startPath.block ),
376+
endAtInnerBoundary = rangeAtInnerBlockBoundary( range, endPath.block, 1 );
365377

366378
// Shrink the range to exclude harmful "noises" (#4087, #4450, #5435).
367379
range.shrink( CKEDITOR.SHRINK_ELEMENT, true );
368380

381+
if ( startAtInnerBoundary )
382+
range.setStartAt( startPath.block, CKEDITOR.POSITION_BEFORE_END );
383+
if ( endAtInnerBoundary )
384+
range.setEndAt( endPath.block, CKEDITOR.POSITION_AFTER_START );
385+
369386
touchPre = range.endContainer.hasAscendant( 'pre', true ) || range.startContainer.hasAscendant( 'pre', true );
370387

371388
range.enlarge( this.forceBrBreak && !touchPre || !this.enlargeBr ? CKEDITOR.ENLARGE_LIST_ITEM_CONTENTS : CKEDITOR.ENLARGE_BLOCK_CONTENTS );
@@ -489,6 +506,17 @@
489506
return 1;
490507
}
491508

509+
// Checks whether range starts or ends at inner block boundary.
510+
// See usage comments to learn more.
511+
function rangeAtInnerBlockBoundary( range, block, checkEnd ) {
512+
if ( !block )
513+
return false;
514+
515+
var testRange = range.clone();
516+
testRange.collapse( !checkEnd );
517+
return testRange.checkBoundaryOfElement( block, checkEnd ? CKEDITOR.START : CKEDITOR.END );
518+
}
519+
492520
/**
493521
* Creates {CKEDITOR.dom.iterator} instance for this range.
494522
*

‎tests/core/dom/iterator.js

Lines changed: 122 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,21 @@ function iterateScopedRange( html, opt ) {
4646
return { html: tools.compatHtml( sandbox.getOuterHtml() ), list: results };
4747
}
4848

49+
function iterateWithRangeIterator( ranges, mergeConsequent, rangeIteratorOpts ) {
50+
var rangesIterator = ranges.createIterator(),
51+
range,
52+
blockList = [];
53+
54+
while ( range = rangesIterator.getNextRange( mergeConsequent ) ) {
55+
var iter = range.createIterator(), block;
56+
CKEDITOR.tools.extend( iter, rangeIteratorOpts, true );
57+
while ( ( block = iter.getNextParagraph() ) )
58+
blockList.push( block.getName() );
59+
}
60+
61+
return blockList;
62+
}
63+
4964
function checkActiveFilter( source, opt, results, msg ) {
5065
var sandbox = doc.getById( 'sandbox' );
5166
var range = tools.setHtmlWithRange( sandbox, source )[ 0 ];
@@ -91,6 +106,52 @@ bender.test( {
91106
checkRangeIteration( source, null, [ 'p' ], output, 'Iteration will yield one single paragraph' );
92107
},
93108

109+
// #12178
110+
'test iterating over end of line': function() {
111+
if ( !CKEDITOR.env.needsBrFiller )
112+
assert.ignore();
113+
114+
var source = '<h1>para1[<br /></h1><p>par]a2</p>';
115+
checkRangeIteration( source, null, [ 'h1', 'p' ], null, 'Iteration will yield heading and paragraph.' );
116+
},
117+
118+
// #12178
119+
'test iterating over end of line - no bogus br': function() {
120+
var source = '<h1>para1[</h1><p>par]a2</p>';
121+
checkRangeIteration( source, null, [ 'h1', 'p' ], null, 'Iteration will yield heading and paragraph.' );
122+
},
123+
124+
// #12178
125+
'test iterating over start of line': function() {
126+
if ( !CKEDITOR.env.needsBrFiller )
127+
assert.ignore();
128+
129+
var source = '<h1>pa[ra1<br /></h1><p>]para2</p>';
130+
checkRangeIteration( source, null, [ 'h1', 'p' ], null, 'Iteration will yield heading and paragraph.' );
131+
},
132+
133+
// #12178
134+
'test iterating over start of line - no bogus br': function() {
135+
var source = '<h1>pa[ra1</h1><p>]para2</p>';
136+
checkRangeIteration( source, null, [ 'h1', 'p' ], null, 'Iteration will yield heading and paragraph.' );
137+
},
138+
139+
// #12178
140+
'test iterating over start of line 2 - no bogus br': function() {
141+
var source = '<h1>pa[ra1</h1><p><b>]para2</b></p>';
142+
checkRangeIteration( source, null, [ 'h1', 'p' ], null, 'Iteration will yield heading and paragraph.' );
143+
},
144+
145+
// #12178
146+
'test iterating over start of line 2 - no bogus br - rangeIterator': function() {
147+
var source = '<h1>pa[ra1</h1><p><b>]para2</b></p>';
148+
149+
var sandbox = doc.getById( 'sandbox' ),
150+
ranges = tools.setHtmlWithRange( sandbox, source );
151+
152+
assert.areSame( 'h1,p', iterateWithRangeIterator( ranges ).join( ',' ) );
153+
},
154+
94155
'test iterating over pseudo block': function() {
95156
var source = '<div><p>[paragraph</p>text]</div>';
96157
var output = '<div><p>paragraph</p><p>text</p></div>';
@@ -139,31 +200,79 @@ bender.test( {
139200
},
140201

141202
// #6728, #4450
203+
// While this test may seem to be totally broken (why would someone create bookmakrs between <tr> and <td>?)
204+
// it has a deeper sense. It tests what rangeIterator#getNextRange does.
142205
'test iterating over table cells (with bookmarks among cells)': function() {
143206
var source = '<table><tbody><tr>[<td id="cell1">cell1</td>][<td id="cell2">cell2</td>]</tr></tbody></table>',
144207
output = source.replace( /\[|\]|\^/g, '' );
145208

146-
var sandbox = doc.getById( 'sandbox' );
147-
var ranges = tools.setHtmlWithRange( sandbox, source );
209+
var sandbox = doc.getById( 'sandbox' ),
210+
ranges = tools.setHtmlWithRange( sandbox, source );
148211

149-
// Create bookmarks by intention.
212+
// Create bookmarks intentionally.
150213
var bms = ranges.createBookmarks();
151214

152-
// Check iteration sequence.
153-
var rangeIterator = ranges.createIterator(), range, blockList = [];
154-
// Merge multiple ranges.
155-
while ( range = rangeIterator.getNextRange( true ) ) {
156-
var iter = range.createIterator(), block;
157-
while ( ( block = iter.getNextParagraph() ) )
158-
blockList.push( block.getName() );
159-
}
215+
assert.areSame( 'td,td', iterateWithRangeIterator( ranges ).join( ',' ), true );
216+
217+
// Just to remove bookmarks.
218+
ranges.moveToBookmarks( bms );
160219

161-
arrayAssert.itemsAreEqual( [ 'td', 'td' ], blockList );
220+
assert.areSame( tools.compatHtml( output ), tools.compatHtml( sandbox.getHtml() ) );
221+
},
222+
223+
// See above an explanation of this test.
224+
'test iterating over table cells (with bookmarks among cells) - do not merge subsequent ranges': function() {
225+
var source = '<table><tbody><tr>[<td id="cell1">cell1</td>][<td id="cell2">cell2</td>]</tr></tbody></table>',
226+
output = source.replace( /\[|\]|\^/g, '' );
227+
228+
var sandbox = doc.getById( 'sandbox' ),
229+
ranges = tools.setHtmlWithRange( sandbox, source );
230+
231+
// Create bookmarks intentionally.
232+
var bms = ranges.createBookmarks();
233+
234+
assert.areSame( 'td,td', iterateWithRangeIterator( ranges ).join( ',' ) );
162235

163236
// Just to remove bookmarks.
164237
ranges.moveToBookmarks( bms );
165238

166-
assert.areSame( tools.compatHtml( output ) , tools.compatHtml( sandbox.getHtml() ) );
239+
assert.areSame( output, tools.compatHtml( sandbox.getHtml() ) );
240+
},
241+
242+
// #6728, #4450
243+
'test iterating over entire table': function() {
244+
var source = '[<table><tbody><tr><th>cell1</th><td>cell2</td></tr></tbody></table>]',
245+
output1 = source.replace( /\[|\]|\^/g, '' ),
246+
output2 = '<table><tbody><tr><th><p>cell1</p></th><td><p>cell2</p></td></tr></tbody></table>';
247+
248+
checkRangeIteration( source, null, [ 'th', 'td' ], output1, 'Iteration should report table cells' );
249+
checkRangeIteration( source, { enforceRealBlocks: 1 }, [ 'p', 'p' ], output2, 'Iteration should establish paragraphs inside table cells' );
250+
},
251+
252+
// #6728, #4450
253+
'test iterating over entire table - use rangeIterator': function() {
254+
var source = '[<table><tbody><tr><th>cell1</th><td>cell2</td></tr></tbody></table>]',
255+
output = source.replace( /\[|\]|\^/g, '' );
256+
257+
var sandbox = doc.getById( 'sandbox' ),
258+
ranges = tools.setHtmlWithRange( sandbox, source );
259+
260+
assert.areSame( 'th,td', iterateWithRangeIterator( ranges, true ).join( ',' ) );
261+
262+
assert.areSame( output, tools.compatHtml( sandbox.getHtml() ) );
263+
},
264+
265+
// #6728, #4450
266+
'test iterating over entire table - use rangeIterator and enforceRealBlocks': function() {
267+
var source = '[<table><tbody><tr><th>cell1</th><td>cell2</td></tr></tbody></table>]',
268+
output = '<table><tbody><tr><th><p>cell1</p></th><td><p>cell2</p></td></tr></tbody></table>';
269+
270+
var sandbox = doc.getById( 'sandbox' ),
271+
ranges = tools.setHtmlWithRange( sandbox, source );
272+
273+
assert.areSame( 'p,p', iterateWithRangeIterator( ranges, true, { enforceRealBlocks: true } ).join( ',' ) );
274+
275+
assert.areSame( output, tools.compatHtml( sandbox.getHtml() ) );
167276
},
168277

169278
'test iterating over list items': function() {

‎tests/core/dom/range/shrink.js

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,73 @@
179179
assert.isFalse( range.collapsed, 'range.collapsed' );
180180
},
181181

182+
'test shrink does stop on block boundary end - SHRINK_ELEMENT': function() {
183+
var ct = doc.getById( 'editable_playground' ),
184+
source = '<p>para1[</p><p>para2]</p>',
185+
result = '<p>para1</p><p>[para2]</p>';
186+
187+
var range = bender.tools.setHtmlWithRange( ct, source )[ 0 ];
188+
range.shrink( CKEDITOR.SHRINK_ELEMENT, true );
189+
assert.areSame( result, bender.tools.getHtmlWithRanges( ct, new CKEDITOR.dom.rangeList( [ range ] ) ) );
190+
},
191+
192+
'test shrink does stop on block boundary start - SHRINK_ELEMENT': function() {
193+
var ct = doc.getById( 'editable_playground' ),
194+
source = '<p>[para1</p><p>]para2</p>',
195+
result = '<p>[para1]</p><p>para2</p>';
196+
197+
var range = bender.tools.setHtmlWithRange( ct, source )[ 0 ];
198+
range.shrink( CKEDITOR.SHRINK_ELEMENT, true );
199+
assert.areSame( result, bender.tools.getHtmlWithRanges( ct, new CKEDITOR.dom.rangeList( [ range ] ) ) );
200+
},
201+
202+
'test shrink does stop on block boundary end - SHRINK_ELEMENT, no shrink on block boundary': function() {
203+
var ct = doc.getById( 'editable_playground' ),
204+
source = '<p>para1[</p><p>para2]</p>';
205+
206+
var range = bender.tools.setHtmlWithRange( ct, source )[ 0 ];
207+
range.shrink( CKEDITOR.SHRINK_ELEMENT, true, false );
208+
assert.areSame( source, bender.tools.getHtmlWithRanges( ct, new CKEDITOR.dom.rangeList( [ range ] ) ) );
209+
},
210+
211+
'test shrink does stop on block boundary start - SHRINK_ELEMENT, no shrink on block boundary': function() {
212+
var ct = doc.getById( 'editable_playground' ),
213+
source = '<p>[para1</p><p>]para2</p>';
214+
215+
var range = bender.tools.setHtmlWithRange( ct, source )[ 0 ];
216+
range.shrink( CKEDITOR.SHRINK_ELEMENT, true, false );
217+
assert.areSame( source, bender.tools.getHtmlWithRanges( ct, new CKEDITOR.dom.rangeList( [ range ] ) ) );
218+
},
219+
220+
'test shrink does not stop on inline boundary end - SHRINK_ELEMENT': function() {
221+
var ct = doc.getById( 'editable_playground' ),
222+
source = '<p><b>text1[</b><i>te]xt2</i></p>',
223+
result = '<p><b>text1</b><i>[te]xt2</i></p>';
224+
225+
var range = bender.tools.setHtmlWithRange( ct, source )[ 0 ];
226+
range.shrink( CKEDITOR.SHRINK_ELEMENT, true );
227+
assert.areSame( result, bender.tools.getHtmlWithRanges( ct, new CKEDITOR.dom.rangeList( [ range ] ) ) );
228+
},
229+
230+
'test shrink does not stop on inline boundary start - SHRINK_ELEMENT': function() {
231+
var ct = doc.getById( 'editable_playground' ),
232+
source = '<p><b>te[xt1</b><i>]text2</i></p>',
233+
result = '<p><b>te[xt1]</b><i>text2</i></p>';
234+
235+
var range = bender.tools.setHtmlWithRange( ct, source )[ 0 ];
236+
range.shrink( CKEDITOR.SHRINK_ELEMENT, true );
237+
assert.areSame( result, bender.tools.getHtmlWithRanges( ct, new CKEDITOR.dom.rangeList( [ range ] ) ) );
238+
},
239+
240+
'test shrink does stop on text - SHRINK_ELEMENT': function() {
241+
var ct = doc.getById( 'editable_playground' ),
242+
source = '<p><b>text1[</b>x<i>]text2</i></p>';
243+
244+
var range = bender.tools.setHtmlWithRange( ct, source )[ 0 ];
245+
range.shrink( CKEDITOR.SHRINK_ELEMENT, true );
246+
assert.areSame( source, bender.tools.getHtmlWithRanges( ct, new CKEDITOR.dom.rangeList( [ range ] ) ) );
247+
},
248+
182249
'test shrink should stop on a non-editable block - SHRINK_ELEMENT': function() {
183250
var ct = doc.getById( 'editable_playground' ),
184251
source = '[<p contenteditable="false">x</p>]';

0 commit comments

Comments
 (0)