Skip to content
This repository was archived by the owner on Dec 31, 2017. It is now read-only.

Commit a4043ff

Browse files
author
Colin Snover
committed
Stores references to event handlers to prevent memory leaks in IE7. Fixes issues with table cell collapses in IE7. Improves behaviour of input buttons.
1 parent 91ba7f9 commit a4043ff

File tree

3 files changed

+175
-69
lines changed

3 files changed

+175
-69
lines changed

README

+6-3
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,10 @@ RoundRect.run() and you’ve got rounded rectangles!
1010

1111
Features:
1212

13-
* A svelte 4.5kB once minified + gzipped
13+
* A svelte 4.9kB once minified + gzipped
1414
* No dependencies on external libraries
15+
* Functions even if you are using an evil library that modifies
16+
Object.prototype (you know who you are)
1517
* Properly object-oriented; no C masquerading as JavaScript here
1618
* 100% configuration-free border-radius support
1719
* Supports dynamically updating backgrounds and borders, without using polling
@@ -20,10 +22,10 @@ Features:
2022
* Supports input and textarea elements, and even drop-downs in IE8
2123
* Functions properly in IE7 and IE8 strict mode
2224
* (Almost) readable code with (nearly) complete jsdoc documentation
25+
* No memory leaks!
2326

2427
Caveats:
2528

26-
* Leaks memory on INPUT elements.
2729
* Only pixel units are supported for padding, border-width, and border-radius.
2830
Anything else will be assumed to be pixel width, even if it isn’t.
2931
You’ve been warned.
@@ -38,7 +40,8 @@ Caveats:
3840
erattically when the mouse moves over a rounded element. More specifically,
3941
hovering over an area of a rounded element that does not contain a child
4042
element will result in the mouse basically falling through the layer into
41-
VML’s cold, dead arms.
43+
VML’s cold, dead arms. There is a workaround in the code to try to mitigate
44+
this issue but it does not work 100% of the time.
4245
* Could it support more of CSS3 backgrounds and borders, like, perhaps,
4346
rgba backgrounds? HECK YES IT COULD, TOTALLY! …but it doesn’t right now.
4447

roundrect.js

+138-40
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,12 @@
109109
/**
110110
* @type {RegExp}
111111
*/
112-
isNumericString = /^-?\d/;
112+
isNumericString = /^-?\d/,
113+
114+
/**
115+
* @type {string}
116+
*/
117+
hoverClass = ns + '-hover';
113118

114119
/**
115120
* Proxy function.
@@ -167,6 +172,44 @@
167172
return 0;
168173
}
169174

175+
/**
176+
* Determines whether or not an element is a button input.
177+
* @param {Element} element
178+
* @return {boolean}
179+
*/
180+
function isButton(element) {
181+
var tagName, type;
182+
183+
if (!element || !element.nodeName || !element.type) {
184+
return false;
185+
}
186+
187+
tagName = element.nodeName.toUpperCase();
188+
type = element.type.toLowerCase();
189+
190+
return (tagName === 'BUTTON' || (tagName === 'INPUT'
191+
&& (type === 'image' || type === 'button' || type === 'submit' || type === 'reset')));
192+
}
193+
194+
/**
195+
* Determines whether or not an element is a text input.
196+
* @param {Element} element
197+
* @return {boolean}
198+
*/
199+
function isTextField(element) {
200+
var tagName, type;
201+
202+
if (!element || !element.nodeName) {
203+
return false;
204+
}
205+
206+
tagName = element.nodeName.toUpperCase();
207+
type = tagName === 'INPUT' ? element.type.toLowerCase() : null;
208+
209+
return ((tagName === 'INPUT' && (element.type === 'text' || element.type === 'password'))
210+
|| tagName === 'TEXTAREA');
211+
}
212+
170213
/**
171214
* Enables VML on the current page.
172215
*/
@@ -287,8 +330,6 @@
287330
var self = this,
288331
property = window.event.propertyName;
289332

290-
//console.log('PROPCHANGE ' + property);
291-
292333
setTimeout(function () {
293334
self.onPropertyChange.call(self, property);
294335
});
@@ -298,17 +339,18 @@
298339
var self = this,
299340
eventType = window.event.type;
300341

301-
//console.log('STATECHANGE ' + eventType);
302-
303342
setTimeout(function () {
304343
self.onStateChange.call(self, eventType);
305344
});
306345
});
307346

308347
this.onVmlStateChangeProxy = proxy(this, function () {
309-
//console.log('VMLSTATECHANGE' + event.type);
348+
// IE seems to sometimes ditch properties from the event object
349+
// if we do not create references to them here before passing to
350+
// the statechange function
351+
var eventType = window.event.type;
310352

311-
this.onVmlStateChange(window.event.type);
353+
this.onVmlStateChange(eventType);
312354
});
313355

314356
this.events = {
@@ -620,29 +662,30 @@
620662

621663
/**
622664
* Breaks references to the DOM to allow Microsoft’s crap GC to GC.
665+
* (I am not entirely sure how many of this is actually necessary,
666+
* since 1. who cares about IE6, 2. sIEve is actually incredibly
667+
* unreliable at determining leaks, and 3. leaks are fixed in IE8
668+
* and will get collected when navigating to another page in IE7.
669+
* Expert advice is appreciated.)
623670
* @param {boolean=} restoreStyles Whether or not to restore inline
624671
* styles from when the element was first run through RoundRect.
625672
*/
626673
destroy: function (restoreStyles) {
627674
var id = this.element[expando], i;
628-
this.stop();
629675
this.removeEvent();
630676
this.element.removeAttribute(expando);
631677

632678
if (this.vml) {
633679
for (i in this.vml) {
634680
if (this.vml.hasOwnProperty(i)) {
635681
this.vml[i].filler = null;
682+
this.vml[i] = null;
636683
}
637684
}
638685
}
639686

640687
if (this.container) {
641-
// IE will leak orphan nodes if we do not empty out the
642-
// innerHTML before calling removeChild
643-
this.container.innerHTML = '';
644688
if (this.container && this.container.parentNode) {
645-
this.container.parentNode.insertBefore(this.element, this.container);
646689
this.container.parentNode.removeChild(this.container);
647690
}
648691
}
@@ -653,8 +696,11 @@
653696
this.element.style[i] = this.originalStyles[i];
654697
}
655698
}
699+
700+
this.element.parentNode.style.width = '';
656701
}
657702

703+
this.container = null;
658704
this.element = null;
659705
delete collection[id];
660706
},
@@ -710,6 +756,30 @@
710756
this[method](c, 'click', vcp);
711757
},
712758

759+
/**
760+
* Add a class to the element referenced by this RoundRect object.
761+
* @type {string} className
762+
*/
763+
addClass: function (className) {
764+
var oldClassName = ' ' + this.element.className + ' ';
765+
766+
if (oldClassName.indexOf(' ' + className + ' ') === -1) {
767+
this.element.className += ' ' + className;
768+
}
769+
},
770+
771+
/**
772+
* Remove a class from the element referenced by this RoundRect object.
773+
* @type {string} className
774+
*/
775+
removeClass: function (className) {
776+
var oldClassName = ' ' + this.element.className + ' ';
777+
778+
if (oldClassName.indexOf(' ' + className + ' ') !== -1) {
779+
this.element.className = oldClassName.replace(' ' + ns + '-hover ', ' ').replace(/^\s+|\s+$/g, '');
780+
}
781+
},
782+
713783
/**
714784
* Proxy for onVmlStateChange, binds ‘this’. Defined in the
715785
* constructor.
@@ -725,20 +795,32 @@
725795
* @param {string} eventType
726796
*/
727797
onVmlStateChange: function (eventType) {
728-
var className = ' ' + this.element.className + ' ';
729-
730-
if (eventType === 'click'
798+
if (eventType === 'click' && isTextField(this.element)) {
799+
// With RoundRect applied, text inputs can only be
800+
// clicked on where text has already been written. This
801+
// partially works around this issue. It is not perfect:
802+
// clicking empty lines in textareas, for instance, puts the
803+
// carat in the wrong place, but it works in most common cases
804+
// and is much better than the default behaviour.
805+
var range = this.element.createTextRange();
806+
range.moveStart('textedit');
807+
range.select();
808+
}
809+
else if (eventType === 'click' && isButton(this.element)) {
810+
// Much like text fields, clicking on the VML part of a button
811+
// will not trigger the button click
812+
this.element.click();
813+
}
814+
else if (eventType === 'click'
731815
&& document.activeElement !== this.element
732816
&& document.activeElement !== document.body) {
733817
document.activeElement.blur();
734818
}
735-
else if (eventType === 'mouseover'
736-
&& className.indexOf(' ' + ns + '-hover ') === -1) {
737-
this.element.className += ' ' + ns + '-hover';
819+
else if (eventType === 'mouseover') {
820+
this.addClass(hoverClass);
738821
}
739-
else if (eventType === 'mouseout'
740-
&& className.indexOf(' ' + ns + '-hover ') !== -1) {
741-
this.element.className = className.replace(' ' + ns + '-hover ', ' ').replace(/^\s+|\s+$/g, '');
822+
else if (eventType === 'mouseout') {
823+
this.removeClass(hoverClass);
742824
}
743825
},
744826

@@ -839,6 +921,19 @@
839921
}
840922
}
841923
else {
924+
// Buttons always fail to change their hover states properly;
925+
// though maybe it is just because it is really slow?
926+
// TODO: Borders width/colour doesn’t seem to update properly
927+
// even with this change for some reason.
928+
if (isButton(this.element)) {
929+
if (eventType === 'mouseenter') {
930+
this.addClass(hoverClass);
931+
}
932+
else if (eventType === 'mouseleave') {
933+
this.removeClass(hoverClass);
934+
}
935+
}
936+
842937
this.element.runtimeStyle.cssText = '';
843938
this.dimensions = this.calculateDimensions();
844939
this.borderWidths = this.calculateBorderWidths();
@@ -1007,22 +1102,14 @@
10071102
if (tagName === 'IMG') {
10081103
e.style.visibility = 'hidden';
10091104
}
1010-
else if (tagName === 'INPUT' || tagName === 'TEXTAREA') {
1011-
// With RoundRect applied, text inputs can only be
1012-
// clicked on where text has already been written. This
1013-
// partially works around this issue. It is not perfect:
1014-
// clicking empty lines in textareas, for instance, puts the
1015-
// carat in the wrong place, but it works in most common cases
1016-
// and is much better than the default behaviour.
1105+
else if (isTextField(e)) {
10171106
this.container.style.cursor = cs.cursor === 'auto' ? 'text' : cs.cursor;
1018-
this.addEvent('container', 'click', proxy(this, function () {
1019-
var range = this.element.createTextRange();
1020-
range.moveStart('textedit');
1021-
range.select();
1022-
}));
1107+
}
1108+
else if (isButton(e)) {
1109+
this.container.style.cursor = cs.cursor === 'auto' ? 'pointer' : cs.cursor;
10231110
}
10241111

1025-
// Without a timeout, IE will throw “unspecified errors”
1112+
// Without a timeout, IE will throw “unspecified error”s
10261113
setTimeout(proxy(this, function () {
10271114
this.addEvent('container', 'mouseenter', proxy(this, function () {
10281115
var fakeEvent = document.createEventObject(window.event);
@@ -1041,7 +1128,10 @@
10411128
},
10421129

10431130
/**
1044-
* Applies all changes to the VML elements for an element.
1131+
* Applies all changes to the VML elements for an element. You can call
1132+
* this if you are not using RoundRect’s dynamic properties
1133+
* functionality and need to update the style, or if it doesn’t work
1134+
* properly for some reason.
10451135
*/
10461136
applyVML: function () {
10471137
// If the element thinks it is invisible, chances are it was
@@ -1102,7 +1192,8 @@
11021192
var dimensions = this.dimensions,
11031193
i,
11041194
vml = this.vml,
1105-
multiplier;
1195+
multiplier,
1196+
parent = this.element.parentNode;
11061197

11071198
/**
11081199
* Copies style properties from the dimensions hash to another
@@ -1127,6 +1218,15 @@
11271218

11281219
assign(this.container, false);
11291220

1221+
// IE7 inappropriately collapses table cells and gives outlandish
1222+
// values for offsetWidth
1223+
if (!ie8 && (parent.nodeName.toUpperCase() === 'TD' || parent.nodeName.toUpperCase() === 'TH')) {
1224+
parent.style.width = '';
1225+
if (parent.currentStyle.width === 'auto') {
1226+
parent.style.width = dimensions.width + 'px';
1227+
}
1228+
}
1229+
11301230
// I don’t know what this was *supposed* to do, but it seems to
11311231
// just fuck up the borders.
11321232
/*if (ie8) {
@@ -1278,10 +1378,8 @@
12781378
if (imageMap[vmlBg] === undefined) {
12791379
img = new Image();
12801380
img.attachEvent('onload', proxy(this, function () {
1281-
// img.detachEvent('onload', arguments.callee.callee);
1282-
1283-
// Replace the object in the map with something more
1284-
// primitive to save memory
1381+
// Replace the Image object in the map with something
1382+
// more primitive to save memory
12851383
imageMap[vmlBg] = {
12861384
width: this.width,
12871385
height: this.height

0 commit comments

Comments
 (0)