Skip to content

Commit 5453c6b

Browse files
committedAug 3, 2016
Fix(main, taBind, textAngularSetup, globals, taTools.spec): Immproved the issues with html and model getting out of sync
Also corrected issue under Firefox where the initial selection was before all the content - globals: improved stripHtmlToText() slightly - main: fixed a mistyped if in switchView() - taBind: small formatting changes for clarity improved element.on('keyup') code element.on('focus') now detets an bad selection condition under Firefox and fixes this - textAngularSetup: added a coverage comment in recursiveRemoveClass() that is now needed because of changes to taTools.spec - taTools.spec: Changed the test set 'test clear button' so that now the whole htmlcontent is now never selected. This was an issue beacuse when all the htmlcontent was selected this triggered the element.on('focus') fix for Firefox and broke the tests here. All the tests now run properly. The biggest change was to now wrap the test content in a <div class='test-class'></div> which caused the expected values to change.
1 parent 243525d commit 5453c6b

File tree

5 files changed

+39
-26
lines changed

5 files changed

+39
-26
lines changed
 

‎src/globals.js

-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ function stripHtmlToText(html)
5454
var tmp = document.createElement("DIV");
5555
tmp.innerHTML = html;
5656
var res = tmp.textContent || tmp.innerText || "";
57-
res = res.replace(/\n/, "");
5857
return res.trim();
5958
}
6059
// get html

‎src/main.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ textAngular.directive("textAngular", [
397397
//Show the HTML view
398398
var _model;
399399
/* istanbul ignore next: ngModel exists check */
400-
if (attrs.ngModell) {
400+
if (ngModel) {
401401
_model = ngModel.$viewValue;
402402
} else {
403403
_model = scope.html;

‎src/taBind.js

+22-17
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,9 @@ angular.module('textAngular.taBind', ['textAngular.factories', 'textAngular.DOM'
220220
return value;
221221
};
222222

223-
if(attrs.taPaste) _pasteHandler = $parse(attrs.taPaste);
223+
if(attrs.taPaste) {
224+
_pasteHandler = $parse(attrs.taPaste);
225+
}
224226

225227
element.addClass('ta-bind');
226228

@@ -296,8 +298,12 @@ angular.module('textAngular.taBind', ['textAngular.factories', 'textAngular.DOM'
296298

297299
// in here we are undoing the converts used elsewhere to prevent the < > and & being displayed when they shouldn't in the code.
298300
var _compileHtml = function(){
299-
if(_isContentEditable) return element[0].innerHTML;
300-
if(_isInputFriendly) return element.val();
301+
if(_isContentEditable) {
302+
return element[0].innerHTML;
303+
}
304+
if(_isInputFriendly) {
305+
return element.val();
306+
}
301307
throw ('textAngular Error: attempting to update non-editable taBind');
302308
};
303309

@@ -823,23 +829,9 @@ angular.module('textAngular.taBind', ['textAngular.factories', 'textAngular.DOM'
823829
}
824830
}
825831
var val = _compileHtml();
826-
/* istanbul ignore next: FF specific bug fix */
827-
if (val==='<br>') {
828-
// on Firefox this comes back sometimes as <br> which is strange, so we remove this here
829-
//console.log('*************BAD****');
830-
val='';
831-
}
832832
if(_defaultVal !== '' && val.trim() === ''){
833833
_setInnerHTML(_defaultVal);
834834
taSelection.setSelectionToElementStart(element.children()[0]);
835-
}else if(val.substring(0, 1) !== '<' && attrs.taDefaultWrap !== ''){
836-
/* we no longer do this, since there can be comments here and white space
837-
var _savedSelection = rangy.saveSelection();
838-
val = _compileHtml();
839-
val = "<" + attrs.taDefaultWrap + ">" + val + "</" + attrs.taDefaultWrap + ">";
840-
_setInnerHTML(val);
841-
rangy.restoreSelection(_savedSelection);
842-
*/
843835
}
844836
var triggerUndo = _lastKey !== event.keyCode && UNDO_TRIGGER_KEYS.test(event.keyCode);
845837
if(_keyupTimeout) $timeout.cancel(_keyupTimeout);
@@ -876,6 +868,19 @@ angular.module('textAngular.taBind', ['textAngular.factories', 'textAngular.DOM'
876868
element.on('focus', scope.events.focus = function(){
877869
_focussed = true;
878870
element.removeClass('placeholder-text');
871+
/* istanbul ignore next: this is only triggered under Firefox and rare! */
872+
try {
873+
var _cont = taSelection.getSelectionElement();
874+
// in Firefox, there is an issue that the selection is initially set to the
875+
// whole container! And when that happens the first character is inserted before
876+
// the <p><br></p> which is bad
877+
// So we detect the whole container selected and then reset the selection to the
878+
// <p>
879+
if (_cont.tagName.toLowerCase() === 'div' && _cont.id === scope.displayElements.text.attr('id')) {
880+
// opps we are actually selecting the whole container!
881+
taSelection.setSelectionToElementStart(_cont.firstChild);
882+
}
883+
}catch(e){}
879884
_reApplyOnSelectorHandlers();
880885
});
881886

‎src/textAngularSetup.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -711,7 +711,10 @@ angular.module('textAngularSetup', [])
711711
var $editor = this.$editor();
712712
var recursiveRemoveClass = function(node){
713713
node = angular.element(node);
714-
if(node[0] !== $editor.displayElements.text[0]) node.removeAttr('class');
714+
/* istanbul ignore next: this is not triggered in tests any longer since we now never select the whole displayELement */
715+
if(node[0] !== $editor.displayElements.text[0]) {
716+
node.removeAttr('class');
717+
}
715718
angular.forEach(node.children(), recursiveRemoveClass);
716719
};
717720
angular.forEach(possibleNodes, recursiveRemoveClass);

‎test/taTools.spec.js

+12-6
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,7 @@ describe('taTools test tool actions', function(){
406406
$window = _$window_;
407407
$window.prompt = function(){ return ''; };
408408
$rootScope = _$rootScope_;
409-
$rootScope.htmlcontent = '<p class="test-class" style="text-align: left;">Test Content <b>that</b> <u>should</u> be cleared</p><h1>Test Other Tags</h1><ul><li>Test <b>1</b></li><li>Test 2</li></ul>';
409+
$rootScope.htmlcontent = '<div class="test-class"><p class="test-class" style="text-align: left;">Test Content <b>that</b> <u>should</u> be cleared</p><h1>Test Other Tags</h1><ul><li>Test <b>1</b></li><li>Test 2</li></ul></div>';
410410
element = _$compile_('<text-angular name="testclearbutton" ng-model="htmlcontent"></text-angular>')($rootScope);
411411
$document.find('body').append(element);
412412
$rootScope.$digest();
@@ -428,10 +428,16 @@ describe('taTools test tool actions', function(){
428428
});
429429

430430
it('clears out all formatting', function(){
431+
var sel = $window.rangy.getSelection();
432+
var range = $window.rangy.createRangyRange();
433+
range.selectNode(jQuery('div.test-class')[0]);
434+
sel.setSingleRange(range);
435+
sel.refresh();
436+
//console.log(sel);
431437
findAndTriggerButton('clear');
432438
//expect($rootScope.htmlcontent).toBe('<p>Test Content that should be cleared</p><p>Test Other Tags</p><p>Test 1</p><p>Test 2</p>');
433439
// bug in phantom JS
434-
expect($rootScope.htmlcontent).toBe('<p>Test Content that should be cleared</p><h1>Test Other Tags</h1><p>Test 1</p><p>Test 2</p>');
440+
expect($rootScope.htmlcontent).toBe('<div><p>Test Content that should be cleared</p><h1>Test Other Tags</h1><p>Test 1</p><p>Test 2</p></div>');
435441
});
436442

437443
it('doesn\'t remove partially selected list elements, but clears them of formatting', function(){
@@ -442,7 +448,7 @@ describe('taTools test tool actions', function(){
442448
sel.setSingleRange(range);
443449
sel.refresh();
444450
findAndTriggerButton('clear');
445-
expect($rootScope.htmlcontent).toBe('<p class="test-class" style="text-align: left;">Test Content <b>that</b> <u>should</u> be cleared</p><h1>Test Other Tags</h1><ul><li>Test 1</li><li>Test 2</li></ul>');
451+
expect($rootScope.htmlcontent).toBe('<div class="test-class"><p class="test-class" style="text-align: left;">Test Content <b>that</b> <u>should</u> be cleared</p><h1>Test Other Tags</h1><ul><li>Test 1</li><li>Test 2</li></ul></div>');
446452
});
447453

448454
it('doesn\'t clear wholly selected list elements, but clears them of formatting', function(){
@@ -452,7 +458,7 @@ describe('taTools test tool actions', function(){
452458
sel.setSingleRange(range);
453459
sel.refresh();
454460
findAndTriggerButton('clear');
455-
expect($rootScope.htmlcontent).toBe('<p class="test-class" style="text-align: left;">Test Content <b>that</b> <u>should</u> be cleared</p><h1>Test Other Tags</h1><ul><li>Test 1</li><li>Test 2</li></ul>');
461+
expect($rootScope.htmlcontent).toBe('<div class="test-class"><p class="test-class" style="text-align: left;">Test Content <b>that</b> <u>should</u> be cleared</p><h1>Test Other Tags</h1><ul><li>Test 1</li><li>Test 2</li></ul></div>');
456462
});
457463

458464
it('doesn\'t clear singly selected list elements, but clears them of formatting', function(){
@@ -463,7 +469,7 @@ describe('taTools test tool actions', function(){
463469
sel.setSingleRange(range);
464470
sel.refresh();
465471
findAndTriggerButton('clear');
466-
expect($rootScope.htmlcontent).toBe('<p class="test-class" style="text-align: left;">Test Content <b>that</b> <u>should</u> be cleared</p><h1>Test Other Tags</h1><ul><li>Test 1</li><li>Test 2</li></ul>');
472+
expect($rootScope.htmlcontent).toBe('<div class="test-class"><p class="test-class" style="text-align: left;">Test Content <b>that</b> <u>should</u> be cleared</p><h1>Test Other Tags</h1><ul><li>Test 1</li><li>Test 2</li></ul></div>');
467473
});
468474

469475
it('doesn\'t clear singly selected list elements, but clears them of formatting', function(){
@@ -474,7 +480,7 @@ describe('taTools test tool actions', function(){
474480
sel.setSingleRange(range);
475481
sel.refresh();
476482
findAndTriggerButton('clear');
477-
expect($rootScope.htmlcontent).toBe('<p class="test-class" style="text-align: left;">Test Content <b>that</b> <u>should</u> be cleared</p><h1>Test Other Tags</h1><ul><li>Test 1</li><li>Test 2</li></ul>');
483+
expect($rootScope.htmlcontent).toBe('<div class="test-class"><p class="test-class" style="text-align: left;">Test Content <b>that</b> <u>should</u> be cleared</p><h1>Test Other Tags</h1><ul><li>Test 1</li><li>Test 2</li></ul></div>');
478484
});
479485

480486
describe('collapsed selection in list escapse list element', function(){

0 commit comments

Comments
 (0)