Merge pull request #2452 from ckeditor/t/2394

Fixed emoji text matching with repeated symbols in a single line
ckeditor · Oct 3, 2018 · 3c9c65e · 3c9c65e
2 parents 4f77f5e + 2850ae1
commit 3c9c65e
Show file tree

Hide file tree

Showing 5 changed files with 67 additions and 43 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -16,6 +16,7 @@ New Features:
 Fixed Issues:
 
 * [#1477](/~https://github.com/ckeditor/ckeditor-dev/issues/1477): Fixed: [Balloon Toolbar](https://ckeditor.com/cke4/addon/balloontoolbar) on destroy doesn't destroy its content.
+* [#2394](/~https://github.com/ckeditor/ckeditor-dev/issues/2394): Fixed: [Emoji](https://ckeditor.com/cke4/addon/emoji) dropdown doesn't show up with repeated symbols in a single line.
 
 API Changes:
 

diff --git a/plugins/emoji/plugin.js b/plugins/emoji/plugin.js
@@ -74,8 +74,8 @@
 						return null;
 					}
 
-					// In case of space preceding colon we need to return index of capturing grup.
-					return { start: text.indexOf( match[ 1 ] ), end: offset };
+					// In case of space preceding colon we need to return the last index (#2394) of capturing grup.
+					return { start: left.lastIndexOf( match[ 1 ] ), end: offset };
 				}
 
 				function dataCallback( matchInfo, callback ) {

diff --git a/tests/plugins/emoji/basic.js b/tests/plugins/emoji/basic.js
@@ -29,7 +29,13 @@
 		}
 	};
 
-	var stub = null;
+	// Disable autocomplete throttling so the tests can be run synchronously.
+	var throttle = CKEDITOR.tools.buffers.throttle,
+		stub = null;
+
+	sinon.stub( CKEDITOR.tools.buffers, 'throttle', function( minInterval, output, contextObj ) {
+		return new throttle( 0, output, contextObj );
+	} );
 
 	var singleTests = {
 		'test for custom emoji characters': function() {
@@ -46,18 +52,12 @@
 				emojiTools.assertIsNullOrUndefined( autocomplete.model.query );
 				emojiTools.assertIsNullOrUndefined( autocomplete.model.data );
 
-				// Handle throttle in autocomplete which by defualt is 20ms;
-				setTimeout( function() {
-					resume( function() {
-						bot.setHtmlWithSelection( '<p>foo :dagg^</p>' );
-						editor.editable().fire( 'keyup', new CKEDITOR.dom.event( {} ) );
-						assert.areSame( ':dagg', autocomplete.model.query, 'Model keeps wrong querry.' );
-						assert.areSame( 1, autocomplete.model.data.length, 'Emoji result contains more than one result.' );
-						objectAssert.areEqual( { id: ':dagger:', symbol: '🗡' }, autocomplete.model.data[ 0 ], 'Emoji result contains wrong result' );
-						autocomplete.close();
-					} );
-				}, 50 );
-				wait();
+				bot.setHtmlWithSelection( '<p>foo :dagg^</p>' );
+				editor.editable().fire( 'keyup', new CKEDITOR.dom.event( {} ) );
+				assert.areSame( ':dagg', autocomplete.model.query, 'Model keeps wrong query' );
+				assert.areSame( 1, autocomplete.model.data.length, 'Emoji result contains more than one result' );
+				objectAssert.areEqual( { id: ':dagger:', symbol: '🗡' }, autocomplete.model.data[ 0 ], 'Emoji result contains wrong result' );
+				autocomplete.close();
 			} );
 		}
 	};
@@ -100,8 +100,8 @@
 
 		'test emoji objects are added to editor': function( editor ) {
 			emojiTools.runAfterInstanceReady( editor, null, function( editor ) {
-				assert.isObject( editor._.emoji, 'Emoji variable doesn\' exists.' );
-				objectAssert.ownsKeys( [ 'list', 'autocomplete' ], editor._.emoji, 'Emoji variable is missing some keys.' );
+				assert.isObject( editor._.emoji, 'Emoji variable doesn\' exists' );
+				objectAssert.ownsKeys( [ 'list', 'autocomplete' ], editor._.emoji, 'Emoji variable is missing some keys' );
 			} );
 		},
 
@@ -111,8 +111,8 @@
 
 				bot.setHtmlWithSelection( '<p>foo :bug^</p>' );
 				editor.editable().fire( 'keyup', new CKEDITOR.dom.event( {} ) );
-				assert.areSame( ':bug', autocomplete.model.query, 'Model keeps wrong querry.' );
-				assert.areSame( 1, autocomplete.model.data.length, 'Emoji result contains more than one result.' );
+				assert.areSame( ':bug', autocomplete.model.query, 'Model keeps wrong query' );
+				assert.areSame( 1, autocomplete.model.data.length, 'Emoji result contains more than one result' );
 				objectAssert.areEqual( { id: ':bug:', symbol: '🐛' }, autocomplete.model.data[ 0 ], 'Emoji result contains wrong result' );
 			} );
 		},
@@ -136,15 +136,11 @@
 				var autocomplete = editor._.emoji.autocomplete,
 					queries = [ ':OK_HAND', ':ok_hand', ':OK_hand', ':ok_HAND', ':Ok_hanD', 'oK_HANd' ];
 
-				CKEDITOR.tools.array.forEach( queries, function( query, index ) {
-					setTimeout( function() {
-						resume();
-						bot.setHtmlWithSelection( '<p>foo ' + query + '^</p>' );
-						editor.editable().fire( 'keyup', new CKEDITOR.dom.event( {} ) );
+				CKEDITOR.tools.array.forEach( queries, function( query ) {
+					bot.setHtmlWithSelection( '<p>foo ' + query + '^</p>' );
+					editor.editable().fire( 'keyup', new CKEDITOR.dom.event( {} ) );
 
-						objectAssert.areEqual( { id: ':ok_hand:', symbol: '👌' }, autocomplete.model.data[ 0 ], 'Emoji result contains wrong result' );
-					}, 50 * ( index + 1 ) );
-					wait();
+					objectAssert.areEqual( { id: ':ok_hand:', symbol: '👌' }, autocomplete.model.data[ 0 ], 'Emoji result contains wrong result' );
 				} );
 			} );
 		},
@@ -157,16 +153,10 @@
 
 				bot.setHtmlWithSelection( '<p>foo:bug^</p>' );
 
-				// Delay assertions because of autocomplete throttle.
-				setTimeout( function() {
-					resume( function() {
-						emojiTools.assertIsNullOrUndefined( autocomplete.model.query );
-						emojiTools.assertIsNullOrUndefined( autocomplete.model.data );
-					} );
-				}, 50 );
+				emojiTools.assertIsNullOrUndefined( autocomplete.model.query );
+				emojiTools.assertIsNullOrUndefined( autocomplete.model.data );
 
 				editable.fire( 'keyup', new CKEDITOR.dom.event( {} ) );
-				wait();
 			} );
 		},
 
@@ -177,16 +167,21 @@
 					editable = editor.editable();
 
 				bot.setHtmlWithSelection( '<p>foo</p><p>:bug^</p>' );
+				editable.fire( 'keyup', new CKEDITOR.dom.event( {} ) );
+				objectAssert.areEqual( { id: ':bug:', symbol: '🐛' }, autocomplete.model.data[ 0 ], 'Emoji result contains wrong result' );
+			} );
+		},
 
-				// Delay assertions because of autocomplete throttle.
-				setTimeout( function() {
-					resume( function() {
-						objectAssert.areEqual( { id: ':bug:', symbol: '🐛' }, autocomplete.model.data[ 0 ], 'Emoji result contains wrong result' );
-					} );
-				}, 50 );
+		// (#2394)
+		'test emoji correctly matches repeated keywords': function( editor, bot ) {
+			emojiTools.runAfterInstanceReady( editor, bot, function( editor, bot ) {
+				var autocomplete = editor._.emoji.autocomplete;
 
-				editable.fire( 'keyup', new CKEDITOR.dom.event( {} ) );
-				wait();
+				bot.setHtmlWithSelection( '<p>foo :collision :collision^</p>' );
+				editor.editable().fire( 'keyup', new CKEDITOR.dom.event( {} ) );
+				assert.areSame( ':collision', autocomplete.model.query, 'Model keeps wrong query' );
+				assert.areSame( 1, autocomplete.model.data.length, 'Emoji result contains more than one result' );
+				objectAssert.areEqual( { id: ':collision:', symbol: '💥' }, autocomplete.model.data[ 0 ], 'Emoji result contains wrong result' );
 			} );
 		}
 	};

diff --git a/tests/plugins/emoji/manual/repeated.html b/tests/plugins/emoji/manual/repeated.html
@@ -0,0 +1,10 @@
+<h2>Classic editor</h2>
+<div id="classic"></div>
+
+<h2>Inline editor</h2>
+<div id="inline" contenteditable="true"></div>
+
+<script>
+	CKEDITOR.replace( 'classic' );
+	CKEDITOR.inline( 'inline' );
+</script>
diff --git a/tests/plugins/emoji/manual/repeated.md b/tests/plugins/emoji/manual/repeated.md
@@ -0,0 +1,18 @@
+@bender-tags: 4.11.0, bug, emoji, 2394
+@bender-ckeditor-plugins: wysiwygarea, sourcearea, emoji
+@bender-ui: collapsed
+@bender-include: ../_helpers/tools.js
+
+## For both editors:
+
+1. Focus the editor.
+1. Type `:bu :bu`.
+
+### Expected
+
+Dropdown shows up with `:bug` (🐛) suggestion symbol.
+
+### Unexpected
+
+* Dropdown doesn't appear.
+* Dropdown contains different sugestion symbols.