Skip to content
This repository was archived by the owner on Sep 5, 2024. It is now read-only.

Commit 0afbbd0

Browse files
devversionThomasBurleson
authored andcommitted
fix(icon): rename nodes id's from cache to prevent duplicates.
Currently we always rename the icons id's on cache initialization, but that causes problems, when using the cached icon multiple times in the app. A super elegant solution, is to transform the ids, when the icon is requested from the cache. Closes #7468
1 parent f5f9177 commit 0afbbd0

File tree

2 files changed

+56
-22
lines changed

2 files changed

+56
-22
lines changed

src/components/icon/icon.spec.js

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,41 @@ describe('mdIcon service', function() {
478478
});
479479
});
480480

481+
describe('icon is cached', function() {
482+
483+
it('should prevent duplicate ids', function() {
484+
var firstId;
485+
486+
$mdIcon('android.svg').then(function(el) {
487+
// First child is in our case always the node with an id.
488+
firstId = el.firstChild.id;
489+
});
490+
491+
$scope.$digest();
492+
493+
$mdIcon('android.svg').then(function(el) {
494+
expect(el.firstChild.id).not.toBe(firstId);
495+
});
496+
497+
$scope.$digest();
498+
499+
});
500+
501+
it('should suffix duplicated ids', function() {
502+
// Just request the icon to be stored in the cache.
503+
$mdIcon('android.svg');
504+
505+
$scope.$digest();
506+
507+
$mdIcon('android.svg').then(function(el) {
508+
expect(el.firstChild.id).toMatch(/.+_cache[0-9]+/g);
509+
});
510+
511+
$scope.$digest();
512+
});
513+
514+
});
515+
481516
describe('icon group is not found', function() {
482517
it('should log Error', function() {
483518
var msg;
@@ -509,11 +544,6 @@ describe('mdIcon service', function() {
509544
function updateDefaults(svg) {
510545
svg = angular.element(svg)[0];
511546

512-
svg.removeAttribute('id');
513-
angular.forEach(svg.querySelectorAll('[id]'), function(item) {
514-
item.removeAttribute('id');
515-
});
516-
517547
angular.forEach({
518548
'xmlns' : 'http://www.w3.org/2000/svg',
519549
'fit' : '',

src/components/icon/js/iconService.js

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -316,9 +316,9 @@
316316

317317
},
318318

319-
$get : ['$http', '$q', '$log', '$templateCache', function($http, $q, $log, $templateCache) {
319+
$get : ['$http', '$q', '$log', '$templateCache', '$mdUtil', function($http, $q, $log, $templateCache, $mdUtil) {
320320
this.preloadIcons($templateCache);
321-
return MdIconService(config, $http, $q, $log, $templateCache);
321+
return MdIconService(config, $http, $q, $log, $templateCache, $mdUtil);
322322
}]
323323
};
324324

@@ -373,7 +373,7 @@
373373
*/
374374

375375
/* @ngInject */
376-
function MdIconService(config, $http, $q, $log, $templateCache) {
376+
function MdIconService(config, $http, $q, $log, $templateCache, $mdUtil) {
377377
var iconCache = {};
378378
var urlRegex = /[-\w@:%\+.~#?&//=]{2,}\.[a-z]{2,4}\b(\/[-\w@:%\+.~#?&//=]*)?/i;
379379
var dataUrlRegex = /^data:image\/svg\+xml[\s*;\w\-\=]*?(base64)?,(.*)$/i;
@@ -393,7 +393,7 @@
393393
// If already loaded and cached, use a clone of the cached icon.
394394
// Otherwise either load by URL, or lookup in the registry and then load by URL, and cache.
395395

396-
if ( iconCache[id] ) return $q.when( iconCache[id].clone() );
396+
if ( iconCache[id] ) return $q.when( transformClone(iconCache[id]) );
397397
if ( urlRegex.test(id) || dataUrlRegex.test(id) ) return loadByURL(id).then( cacheIcon(id) );
398398
if ( id.indexOf(':') == -1 ) id = '$default:' + id;
399399

@@ -418,24 +418,28 @@
418418
return result;
419419
}
420420

421+
function transformClone(cacheElement) {
422+
var clone = cacheElement.clone();
423+
var cacheSuffix = '_cache' + $mdUtil.nextUid();
424+
425+
// We need to modify for each cached icon the id attributes.
426+
// This is needed because SVG id's are treated as normal DOM ids
427+
// and should not have a duplicated id.
428+
if (clone.id) clone.id += cacheSuffix;
429+
angular.forEach(clone.querySelectorAll('[id]'), function (item) {
430+
item.id += cacheSuffix;
431+
});
432+
433+
return clone;
434+
}
435+
421436
/**
422437
* Prepare and cache the loaded icon for the specified `id`
423438
*/
424439
function cacheIcon( id ) {
425440

426-
return function updateCache( _icon ) {
427-
var icon = isIcon(_icon) ? _icon : new Icon(_icon, config[id]);
428-
429-
//clear id attributes to prevent aria issues
430-
var elem = icon.element;
431-
elem.removeAttribute('id');
432-
433-
angular.forEach(elem.querySelectorAll('[id]'), function(item) {
434-
item.removeAttribute('id');
435-
});
436-
437-
iconCache[id] = icon;
438-
441+
return function updateCache( icon ) {
442+
iconCache[id] = isIcon(icon) ? icon : new Icon(icon, config[id]);
439443

440444
return iconCache[id].clone();
441445
};

0 commit comments

Comments
 (0)