From cab9ebfd5a02e897f802bf6321b8471e4843c5d3 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Tue, 25 Jul 2017 22:01:48 -0700 Subject: [PATCH] perf(jqLite): avoid repeated add/removeAttribute in jqLiteRemoveClass Fixes #16078 Closes #16131 --- src/jqLite.js | 12 +++++++----- test/jqLiteSpec.js | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/jqLite.js b/src/jqLite.js index a9595078148e..3cfae74ad080 100644 --- a/src/jqLite.js +++ b/src/jqLite.js @@ -420,13 +420,15 @@ function jqLiteHasClass(element, selector) { function jqLiteRemoveClass(element, cssClasses) { if (cssClasses && element.setAttribute) { + var existingClasses = (' ' + (element.getAttribute('class') || '') + ' ') + .replace(/[\n\t]/g, ' '); + forEach(cssClasses.split(' '), function(cssClass) { - element.setAttribute('class', trim( - (' ' + (element.getAttribute('class') || '') + ' ') - .replace(/[\n\t]/g, ' ') - .replace(' ' + trim(cssClass) + ' ', ' ')) - ); + cssClass = trim(cssClass); + existingClasses = existingClasses.replace(' ' + cssClass + ' ', ' '); }); + + element.setAttribute('class', trim(existingClasses)); } } diff --git a/test/jqLiteSpec.js b/test/jqLiteSpec.js index 094db572eb01..4a081844d160 100644 --- a/test/jqLiteSpec.js +++ b/test/jqLiteSpec.js @@ -914,6 +914,23 @@ describe('jqLite', function() { }); + // JQLite specific implementation/performance tests + if (_jqLiteMode) { + it('should only get/set the attribute once when multiple classes added', function() { + var fakeElement = { + nodeType: 1, + setAttribute: jasmine.createSpy(), + getAttribute: jasmine.createSpy().and.returnValue('') + }; + var jqA = jqLite(fakeElement); + + jqA.addClass('foo bar baz'); + expect(fakeElement.getAttribute).toHaveBeenCalledOnceWith('class'); + expect(fakeElement.setAttribute).toHaveBeenCalledOnceWith('class', 'foo bar baz'); + }); + } + + it('should not add duplicate classes', function() { var jqA = jqLite(a); expect(a.className).toBe(''); @@ -1031,6 +1048,23 @@ describe('jqLite', function() { jqA.removeClass('foo baz noexistent'); expect(a.className).toBe('bar'); }); + + + // JQLite specific implementation/performance tests + if (_jqLiteMode) { + it('should get/set the attribute once when removing multiple classes', function() { + var fakeElement = { + nodeType: 1, + setAttribute: jasmine.createSpy(), + getAttribute: jasmine.createSpy().and.returnValue('foo bar baz') + }; + var jqA = jqLite(fakeElement); + + jqA.removeClass('foo baz noexistent'); + expect(fakeElement.getAttribute).toHaveBeenCalledOnceWith('class'); + expect(fakeElement.setAttribute).toHaveBeenCalledOnceWith('class', 'bar'); + }); + } }); });