PDA

View Full Version : [DUP] [4.0.2a] Bug in Element.slideIn



westy
20 Jun 2011, 4:56 AM
Hi,

Been seeing issues with animation and older versions of IE ever since Ext 4 launched.
They're a little hard to track what is going on, and how though, and given that it was older versions I just thought it was them being rubbish so disabled it.

The issue is seen mostly when tree animation occurs, normally during an expansion that causes a load, and no, I don't have a canned repeatable test case at the moment ;)

Since 4.0.2a I've been seeing issues in IE9 though, so thought I'd try and look deeper.

Am seeing the following error:
Unable to get value of the property 'insertBefore': object is null or undefined
ext-all-debug.js, line 9351 character 21

The line in question is in Element.anim.js, in the huge slideIn method (which makes it a pain to override, might I add):


if (wrap.dom) {
wrap.dom.parentNode.insertBefore(me.dom, wrap.dom);
wrap.remove();
}


Basically, it seems that sometimes the dom node wrapped by an animation has a null parentNode, but is used blindly.


Worth noting that also seeing the same thing in Firefox, but in a different place, line 7963 of ext-all-debug, which is in Element.insertion.js, insertSibling method (which looks like it has the scope to blow in a couple of places):


if(el.nodeType || el.dom){
rt = me.dom.parentNode.insertBefore(Ext.getDom(el), isAfter ? me.dom.nextSibling : me.dom);
if (!returnDom) {
rt = Ext.get(rt);
}
}else{
if (isAfter && !me.dom.nextSibling) {
rt = Ext.core.DomHelper.append(me.dom.parentNode, el, !returnDom);
} else {
rt = Ext.core.DomHelper[isAfter ? 'insertAfter' : 'insertBefore'](me.dom, el, !returnDom);
}
}



Firstly, why or how can a dom elements parentNode be null? Is it anything I can control?
Secondly, how about some defense around the above usage of the parentNode, i.e. if (blah.parentNode)? What side-effects would that have?
Thirdly, could some of this be made easier to override?

Would welcome some thoughts, and a speedy fix please!
IE unusable at the moment!

Cheers,
Westy

westy
20 Jun 2011, 8:26 AM
Have had to 'override' the slideIn one, since once it happens it appears that no windows can open, collapsing panels no longer works, etc.
Generally, it's bad m'kay.

I say 'override' since had to apply it to Ext.core.Element.prototype, since this is how the class is defined.

My fix surrounded by a WestyFix tag :)


Ext.apply(Ext.core.Element.prototype, {
/*
* Override of this method, to try and protect against a parentNode in the DOM being null.
* See: ATG-391 & http://www.sencha.com/forum/showthread.php?137685-4.0.2a-Bug-in-Element.slideIn&p=616171
* @param {String} anchor (optional) One of the valid Fx anchor positions (defaults to top: 't')
* @param {Object} options (optional) Object literal with any of the Fx config options
* @return {Ext.core.Element} The Element
*/
slideIn: function(anchor, obj, slideOut) {
var me = this,
elStyle = me.dom.style,
beforeAnim, wrapAnim;

anchor = anchor || "t";
obj = obj || {};

beforeAnim = function() {
var animScope = this,
listeners = obj.listeners,
box, position, restoreSize, wrap, anim;

if (!slideOut) {
me.fixDisplay();
}

box = me.getBox();
if ((anchor == 't' || anchor == 'b') && box.height == 0) {
box.height = me.dom.scrollHeight;
}
else if ((anchor == 'l' || anchor == 'r') && box.width == 0) {
box.width = me.dom.scrollWidth;
}

position = me.getPositioning();
me.setSize(box.width, box.height);

wrap = me.wrap({
style: {
visibility: slideOut ? 'visible' : 'hidden'
}
});
wrap.setPositioning(position);
if (wrap.isStyle('position', 'static')) {
wrap.position('relative');
}
me.clearPositioning('auto');
wrap.clip();

// This element is temporarily positioned absolute within its wrapper.
// Restore to its default, CSS-inherited visibility setting.
// We cannot explicitly poke visibility:visible into its style because that overrides the visibility of the wrap.
me.setStyle({
visibility: '',
position: 'absolute'
});
if (slideOut) {
wrap.setSize(box.width, box.height);
}

switch (anchor) {
case 't':
anim = {
from: {
width: box.width + 'px',
height: '0px'
},
to: {
width: box.width + 'px',
height: box.height + 'px'
}
};
elStyle.bottom = '0px';
break;
case 'l':
anim = {
from: {
width: '0px',
height: box.height + 'px'
},
to: {
width: box.width + 'px',
height: box.height + 'px'
}
};
elStyle.right = '0px';
break;
case 'r':
anim = {
from: {
x: box.x + box.width,
width: '0px',
height: box.height + 'px'
},
to: {
x: box.x,
width: box.width + 'px',
height: box.height + 'px'
}
};
break;
case 'b':
anim = {
from: {
y: box.y + box.height,
width: box.width + 'px',
height: '0px'
},
to: {
y: box.y,
width: box.width + 'px',
height: box.height + 'px'
}
};
break;
case 'tl':
anim = {
from: {
x: box.x,
y: box.y,
width: '0px',
height: '0px'
},
to: {
width: box.width + 'px',
height: box.height + 'px'
}
};
elStyle.bottom = '0px';
elStyle.right = '0px';
break;
case 'bl':
anim = {
from: {
x: box.x + box.width,
width: '0px',
height: '0px'
},
to: {
x: box.x,
width: box.width + 'px',
height: box.height + 'px'
}
};
elStyle.right = '0px';
break;
case 'br':
anim = {
from: {
x: box.x + box.width,
y: box.y + box.height,
width: '0px',
height: '0px'
},
to: {
x: box.x,
y: box.y,
width: box.width + 'px',
height: box.height + 'px'
}
};
break;
case 'tr':
anim = {
from: {
y: box.y + box.height,
width: '0px',
height: '0px'
},
to: {
y: box.y,
width: box.width + 'px',
height: box.height + 'px'
}
};
elStyle.bottom = '0px';
break;
}

wrap.show();
wrapAnim = Ext.apply({}, obj);
delete wrapAnim.listeners;
wrapAnim = Ext.create('Ext.fx.Anim', Ext.applyIf(wrapAnim, {
target: wrap,
duration: 500,
easing: 'ease-out',
from: slideOut ? anim.to : anim.from,
to: slideOut ? anim.from : anim.to
}));

// In the absence of a callback, this listener MUST be added first
wrapAnim.on('afteranimate', function() {
if (slideOut) {
me.setPositioning(position);
if (obj.useDisplay) {
me.setDisplayed(false);
} else {
me.hide();
}
}
else {
me.clearPositioning();
me.setPositioning(position);
}
if (wrap.dom) {
// <WestyFix>
if (wrap.dom.parentNode) {
wrap.dom.parentNode.insertBefore(me.dom, wrap.dom);
} else {
// FIXME: No idea what to do here...
}
// </WestyFix>
wrap.remove();
}
me.setSize(box.width, box.height);
animScope.end();
});
// Add configured listeners after
if (listeners) {
wrapAnim.on(listeners);
}
};

me.animate({
duration: obj.duration ? obj.duration * 2 : 1000,
listeners: {
beforeanimate: {
fn: beforeAnim
},
afteranimate: {
fn: function() {
if (wrapAnim && wrapAnim.running) {
wrapAnim.end();
}
}
}
}
});
return me;
}

});

westy
20 Jun 2011, 8:36 AM
Doh, have had to do this one aswell...



/*
* Override of this method, to try and protect against a parentNode in the DOM being null.
* @param {Mixed/Object/Array} el The id, element to insert or a DomHelper config to create and insert *or* an array of any of those.
* @param {String} where (optional) 'before' or 'after' defaults to before
* @param {Boolean} returnDom (optional) True to return the .;ll;l,raw DOM element instead of Ext.core.Element
* @return {Ext.core.Element} The inserted Element. If an array is passed, the last inserted element is returned.
*/
insertSibling: function(el, where, returnDom){
var me = this, rt,
isAfter = (where || 'before').toLowerCase() == 'after',
insertEl;

if(Ext.isArray(el)){
insertEl = me;
Ext.each(el, function(e) {
rt = Ext.fly(insertEl, '_internal').insertSibling(e, where, returnDom);
if(isAfter){
insertEl = rt;
}
});
return rt;
}

el = el || {};

// <WestyFix>
if ((el.nodeType || el.dom) && me.dom.parentNode) {
// </WestyFix>
rt = me.dom.parentNode.insertBefore(Ext.getDom(el), isAfter ? me.dom.nextSibling : me.dom);
if (!returnDom) {
rt = Ext.get(rt);
}
}else{
if (isAfter && !me.dom.nextSibling) {
rt = Ext.core.DomHelper.append(me.dom.parentNode, el, !returnDom);
} else {
rt = Ext.core.DomHelper[isAfter ? 'insertAfter' : 'insertBefore'](me.dom, el, !returnDom);
}
}
return rt;
}

westy
21 Jun 2011, 6:47 AM
Following this issue (http://www.sencha.com/forum/showthread.php?137839-4.0.2a-DomHelper.insertIntoTable&p=616753), I've had to change my insertSibling override to:


insertSibling: function(el, where, returnDom){
var me = this, rt,
isAfter = (where || 'before').toLowerCase() == 'after',
insertEl;

if(Ext.isArray(el)){
insertEl = me;
Ext.each(el, function(e) {
rt = Ext.fly(insertEl, '_internal').insertSibling(e, where, returnDom);
if(isAfter){
insertEl = rt;
}
});
return rt;
}

el = el || {};

// <WestyFix>
if (me.dom.parentNode) {
// </WestyFix>
if(el.nodeType || el.dom){
rt = me.dom.parentNode.insertBefore(Ext.getDom(el), isAfter ? me.dom.nextSibling : me.dom);
if (!returnDom) {
rt = Ext.get(rt);
}
}else{
if (isAfter && !me.dom.nextSibling) {
rt = Ext.core.DomHelper.append(me.dom.parentNode, el, !returnDom);
} else {
rt = Ext.core.DomHelper[isAfter ? 'insertAfter' : 'insertBefore'](me.dom, el, !returnDom);
}
}
}
return rt;
}

westy
3 Aug 2011, 8:19 AM
Still no guard against use of parentNode in 4.0.5

westy
31 Aug 2011, 7:40 AM
...or 4.0.6

edspencer
31 Aug 2011, 6:33 PM
This one's a bit of a wider issue regarding maintaining correct Component state while animating things. I'll ask Don to take a look at your suggestion to see if it's enough to clear this issue type

westy
1 Sep 2011, 1:31 AM
This one's a bit of a wider issue regarding maintaining correct Component state while animating things. I'll ask Don to take a look at your suggestion to see if it's enough to clear this issue type

Nice one, thanks again.

edspencer
1 Sep 2011, 2:31 PM
Hmm I think the above are just symptoms of the real problem so addressing them would be layering on more bandaids. We're discussing this as part of our 4.1 upgrades but it might not be feasible/efficient to attempts to address in 4.0.x...

westy
2 Sep 2011, 1:46 AM
Hmm I think the above are just symptoms of the real problem so addressing them would be layering on more bandaids. We're discussing this as part of our 4.1 upgrades but it might not be feasible/efficient to attempts to address in 4.0.x...

Agreed, these fixes simply work around the fact that parentNode is sometimes undefined.
I'd much rather parentNode was always there, and accessible.

I have to keep the overrides, since cannot have exceptions flying all the time. If there's a better, proper fix in the future then I welcome it.

Whatever you do in 4.1 I implore you to test in old versions of IE (at least 7 & 8), and stuff more complex than many of your examples. Issues tend to crop up very quickly.

Regards,
Westy

edspencer
2 Sep 2011, 10:07 AM
Whatever you do in 4.1 I implore you to test in old versions of IE (at least 7 & 8), and stuff more complex than many of your examples. Issues tend to crop up very quickly.

We're doing a lot of work around that at the moment. It's taken a few iterations but I think we're getting to the right place with our automated visual and performance testing. The system that we're building out runs our full unit test suite, render test suite and (soon) performance test suite on every commit, *before* it's pulled into trunk.

The rendering tests used to just be the examples folder but we're changing that to be a series of gigantic pages with all possible component configurations laid out and compared with a reference. It's going to take a little time to finish building that out but that's our mission. At the moment though it's already enormously more bullet proof than when we shipped 4.0.0. Top 2 priorities at the moment are fixing performance (4.1.x) and clearing out the bug list (4.0.x). Making good progress on both

westy
2 Sep 2011, 10:54 AM
Good to hear you're hard at work, and making progress :)
Hope the new testing strategies catch lots of issues...

kclendinning
23 Mar 2012, 7:46 AM
Did you resolve? any workaround for this error.

I am getting this error when i do a move on my tree nodes too. could you please let me know if there is any work around.

Message: Unable to get value of the property 'insertBefore': object is null or undefined
Line: 15
Char: 159448
Code: 0
URI: http://hello/Manager/extjs/ext-all.js.gz (http://usalvwiaqa3:7201/Manager/extjs/ext-all.js.gz)

westy
23 Mar 2012, 7:53 AM
My workaround is in this thread.

nardo22
2 Mar 2018, 9:48 AM
Ext.override(Ext.dom.Element, {
insertSibling: function(el, where, returnDom) {
var me = this,
DomHelper = Ext.core.DomHelper,
oldUseDom = DomHelper.useDom,
isAfter = (where || 'before').toLowerCase() == 'after',
rt, insertEl, eLen, e;


if (Ext.isArray(el)) {
// append all elements to a documentFragment
insertEl = Ext.fly(document.createDocumentFragment(), '_internal');
eLen = el.length;


// DocumentFragments cannot accept innerHTML
DomHelper.useDom = true;
for (e = 0; e < eLen; e++) {
rt = insertEl.appendChild(el[e], returnDom);
}
DomHelper.useDom = oldUseDom;


// Insert fragment into document
me.dom.parentNode.insertBefore(insertEl.dom, isAfter ? me.dom.nextSibling : me.dom);
return rt;
}


el = el || {};


if (el.nodeType || el.dom) {
rt = me.dom.parentNode.insertBefore(Ext.getDom(el), isAfter ? me.dom.nextSibling : me.dom);
if (!returnDom) {
rt = Ext.get(rt);
}
} else {
if (isAfter && !me.dom.nextSibling) {
rt = DomHelper.append(me.dom.parentNode, el, !returnDom);
} else {
rt = DomHelper[isAfter ? 'insertAfter' : 'insertBefore'](me.dom, el, !returnDom);
}
}
return rt;
}

})