Skip to content

Loading…

Fixes 2129. #2168

Closed
wants to merge 2 commits into from

2 participants

@ibolmo
MooTools member

IE < 9, has issues with caching and getAttribute('width'). If the image
is cached, the width will be specified as soon as you assign the
.src attribute using the cached dimensions. This only happens for
dynamically creates images. IE handles pre-defined elements as expected.

Added Element.src.html spec with instructions on how to test.

Fixed old manual specs that references old Event.js. Updated
Specs/index.html to include older manual specs and the new
Element.src.html.

PASSED: IE6-9

@ibolmo
MooTools member

Fixes #2129.

@ibolmo ibolmo commented on an outdated diff
Source/Element/Element.js
@@ -579,6 +579,19 @@ propertyGetters['class'] = function(node){
return ('className' in node) ? node.className || null : node.getAttribute('class');
};
+/* <ltIE9> */
+if (Browser.ie && Browser.version < 9) propertySetters.src = function(node, value){
+ var isNew = !node.parentNode,
@ibolmo MooTools member
ibolmo added a note

@jdalton this was the only line that I wasn't happy with. Do you know of a better way to check if an Element is "new".

@jdalton
jdalton added a note

Ga really it's 2011, enough with the UA sniffs.

In IE a removed element will still have a parentNode so you can check it against the parentElement property.
(parentElement will be null once detached from the document unlike parentNode)

@ibolmo MooTools member
ibolmo added a note

hehe remember that there's no cheap way to check for image cache bug that we're trying to normalize. I guess I could use Google Analytics blank.gif but what's the point if I know that the behavior is ltIE9.

Thanks for the tip. Do you think that should suffice the isNew? I guess I could temporarily flag it in new Element and toggle it in src.

Oh and I forgot to check against img. I'll do this in a bit.

@jdalton
jdalton added a note

hehe remember that there's no cheap way to check for image cache bug that we're trying to normalize. I guess I could use Google Analytics blank.gif but what's the point if I know that the behavior is ltIE9.

Except the whole thing is a doc note, not a "cache bug", and not really a patch concern for MooTools.
The problem is that unlike other browsers IE will assign height and width attributes automatically for a loaded image. So when you change the src it will enforce the attributes it set. MooTools has no way to determine if the user or IE has set those attributes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
ibolmo added some commits
@ibolmo ibolmo Fixes 2129.
IE < 9, has issues with caching and getAttribute('width'). If the image
is cached, the width will be `specified` as soon as you assign the
`.src` attribute using the cached dimensions. This only happens for
dynamically creates images. IE handles pre-defined elements as expected.

Added `Element.src.html` spec with instructions on how to test.

Fixed old manual specs that references old `Event.js`. Updated
`Specs/index.html` to include older manual specs and the new
`Element.src.html`.

PASSED: IE6-9
e6a7887
@ibolmo ibolmo Simplified fix and adds coverage for edge cases.
Pull-push: what if the element was in DOM and then disposed and reset
src to another. This is buggy since IE resets the width/height even if
the width or height were specified. This fix normalizes.

PASSED: IE6-9
420a28a
@ibolmo
MooTools member

Opting for doc.

@ibolmo ibolmo closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 29, 2012
  1. @ibolmo

    Fixes 2129.

    ibolmo committed
    IE < 9, has issues with caching and getAttribute('width'). If the image
    is cached, the width will be `specified` as soon as you assign the
    `.src` attribute using the cached dimensions. This only happens for
    dynamically creates images. IE handles pre-defined elements as expected.
    
    Added `Element.src.html` spec with instructions on how to test.
    
    Fixed old manual specs that references old `Event.js`. Updated
    `Specs/index.html` to include older manual specs and the new
    `Element.src.html`.
    
    PASSED: IE6-9
  2. @ibolmo

    Simplified fix and adds coverage for edge cases.

    ibolmo committed
    Pull-push: what if the element was in DOM and then disposed and reset
    src to another. This is buggy since IE resets the width/height even if
    the width or height were specified. This fix normalizes.
    
    PASSED: IE6-9
View
17 Source/Element/Element.js
@@ -579,6 +579,23 @@ propertyGetters['class'] = function(node){
return ('className' in node) ? node.className || null : node.getAttribute('class');
};
+/* <ltIE9> */
+if (Browser.ie && Browser.version < 9) propertySetters.src = function(node, value){
+ if (node.get('tag') != 'img') return node.src = value;
+
+ var width = Object.clone(node.getAttributeNode('width')),
+ height = Object.clone(node.getAttributeNode('height'));
+
+ node.src = value;
+
+ if (width.specified) node.style.width = width.nodeValue;
+ if (height.specified) node.style.height = height.nodeValue;
+
+ node.removeAttribute('width');
+ node.removeAttribute('height');
+};
+/* </ltIE9> */
+
/* <webkit> */
var el = document.createElement('button');
// IE sets type as readonly and throws
View
2 Specs/1.4client/Element/Element.Event.change.html
@@ -123,7 +123,7 @@
log('change bubbles', true);
});
</script>
-
+
</div>
</body>
View
153 Specs/1.4client/Element/Element.src.html
@@ -0,0 +1,153 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <meta http-equiv="Content-Type" content="text/html;charset=UTF-8" />
+
+ <title>Element.set('src') specs</title>
+
+ <script src="../../../Source/Core/Core.js" type="text/javascript"></script>
+ <script src="../../../Source/Types/Array.js" type="text/javascript"></script>
+ <script src="../../../Source/Types/Function.js" type="text/javascript"></script>
+ <script src="../../../Source/Types/Number.js" type="text/javascript"></script>
+ <script src="../../../Source/Types/String.js" type="text/javascript"></script>
+ <script src="../../../Source/Types/Object.js" type="text/javascript"></script>
+ <script src="../../../Source/Types/DOMEvent.js" type="text/javascript"></script>
+ <script src="../../../Source/Browser/Browser.js" type="text/javascript"></script>
+ <script src="../../../Source/Slick/Slick.Parser.js" type="text/javascript"></script>
+ <script src="../../../Source/Slick/Slick.Finder.js" type="text/javascript"></script>
+ <script src="../../../Source/Element/Element.js" type="text/javascript"></script>
+ <script src="../../../Source/Element/Element.Event.js" type="text/javascript"></script>
+
+ <style>
+
+ body {
+ font: 13px sans-serif;
+ }
+
+ h1 {
+ border-top: 1px dashed #AAA;
+ font: normal 22px sans-serif;
+ margin: 5px 0;
+ padding-top: 10px;
+ }
+
+ #content {
+ top: 0;
+ bottom: 0;
+ left: 370px;
+ position: absolute;
+ overflow: auto;
+ padding-bottom: 150px;
+ right: 0;
+ }
+
+ #aside {
+ background: #F5F5F5;
+ border-right: 1px dashed #AAA;
+ bottom: 0;
+ font-family: monospace;
+ left: 0;
+ margin-right: 10px;
+ overflow: auto;
+ padding: 10px;
+ position: absolute;
+ top: 0;
+ width: 340px;
+ }
+
+ .success {
+ background: greenyellow;
+ }
+
+ .fail {
+ background: orangered;
+ }
+
+ .box {
+ background: greenyellow;
+ border: 1px solid black;
+ padding: 10px;
+ width: 300px;
+ }
+
+ .note {
+ font-size: small;
+ color: gray;
+ }
+
+ </style>
+
+ </head>
+ <body>
+
+ <div id="aside">
+ <h2>Log</h2>
+ <div id="log"></div>
+ </div>
+
+ <script>
+
+ var logger = $('log');
+ function log(msg, success){
+ new Element('div', {
+ 'class': success == true ? 'success' : (success == false ? 'fail' : ''),
+ text: msg
+ }).inject(logger);
+ };
+
+ </script>
+
+ <div id="content">
+ <h2>Instructions</h2>
+ <ol>
+ <li>Refresh a couple of times so that Internet Explorer caches the image.</li>
+ <li>Click on each image to swap the image with another (much smaller) image.</li>
+ </ol>
+
+ <div id="sandbox">
+ <h3>Original</h3>
+ <img id="original" src="http://blog.accessko.nl/wp-content/uploads/2009/06/logo-mootools.gif" alt="">
+
+ <h3>Styled</h3>
+ <p class="note">Note: this should stay the size specified (90px).</p>
+ <img src="http://blog.accessko.nl/wp-content/uploads/2009/06/logo-mootools.gif" style="width:90px" alt="">
+
+ <h3>Half as large</h3>
+ <p class="note">Note: this should stay the size specified (90px).</p>
+ <img id="half-as-large" width="90" src="http://blog.accessko.nl/wp-content/uploads/2009/06/logo-mootools.gif" alt="">
+
+ <h3>Pull and Push</h3>
+ <p class="note">Note: this should stay the size specified (90px).</p>
+ <div id="target">
+ <img id="pull-push" src="http://blog.accessko.nl/wp-content/uploads/2009/06/logo-mootools.gif" width="90" alt="">
+ </div>
+
+ <h3>Missing src</h3>
+ <img id="missing" alt="">
+
+ <h3>Dynamically Created (this is buggy in IE)</h3>
+ </div>
+
+ <div id="actions">
+ </div>
+
+ <script>
+ new Element('img', {
+ id: 'dynamic',
+ src: 'http://blog.accessko.nl/wp-content/uploads/2009/06/logo-mootools.gif'
+ }).inject('sandbox')
+
+ $('pull-push').dispose().set('src', 'http://www.figureone.com/favicon.png').inject('target');
+
+ $$('#sandbox img').addEvent('click', function(){
+ this.set('src', 'http://www.figureone.com/favicon.png');
+ });
+
+ $('missing').set('src', 'http://www.figureone.com/favicon.png');
+ </script>
+
+ </div>
+
+ </body>
+</html>
+
View
2 Specs/index.html
@@ -21,6 +21,8 @@
</script>
<li><a href="1.3client/Utilities/DOMReady.php">&rarr; 1.3 DOMReady</a></li>
<li><a href="1.4client/Element/Element.Delegation.html">&rarr; 1.4 Delegation</a></li>
+ <li><a href="1.4client/Element/Element.Event.change.html">&rarr; 1.4 Element change event (IE only)</a></li>
+ <li><a href="1.4client/Element/Element.src.html">&rarr; 1.4 Element src (IE only)</a></li>
</ul>
</div>
Something went wrong with that request. Please try again.