Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IE8 and below: "invalid argument" #52

Closed
marqsm opened this issue Jan 3, 2012 · 21 comments
Closed

IE8 and below: "invalid argument" #52

marqsm opened this issue Jan 3, 2012 · 21 comments

Comments

@marqsm
Copy link

marqsm commented Jan 3, 2012

I'm not sure if this is really a bug or not, as it suddenly appeared one day. The component was working fine on IE before. I re-copied the component code so it should be up to date and untouched.

I've run into a situation where IE7-IE8 (probably older ones also, haven't tested) return "invalid argument". With some testing it seems like the failure happens inside ins(parent, child1, child2)-function at

parent.insertBefore(child1, child2||null);

when child2 has a value other than null (on IE8). When error appears, values of the parameters (outerHTML) are for
parent (first and last gt/lt-signs removed as otherwise the example is hidden):

<roundrect style="FILTER: progId:DXImageTransform.Microsoft.Blur(pixelradius=2,makeshadow=1,shadowopacity=.3); WIDTH: 11px; HEIGHT: 4px; TOP: -2px; LEFT: 10px" arcsize="1"></roundrect>

child1:

<fill opacity="0.25" color="#FFFFFF"></fill>

child2:

<stroke opacity="0"></stroke

What came to mind first was is it sure the parent contains the child2? insertBefore without the second parameter works fine.

@debreczeni
Copy link

I am too having the same issue
@marqsm off: you can insert html if you indent the code with 4 spaces and leave a line break before the code block

<roundrect style="FILTER: progId:DXImageTransform.Microsoft.Blur(pixelradius=2,makeshadow=1,shadowopacity=.3); WIDTH: 11px; HEIGHT: 4px; TOP: -2px; LEFT: 10px" arcsize="1"></roundrect>

child1

<fill opacity="0.25" color="#FFFFFF"></fill>

child2

<stroke opacity="0"></stroke>

@marqsm
Copy link
Author

marqsm commented Jan 4, 2012

I managed to fix this by adding some code. This is not the optimal way to do it as checking elements like this isn't all that fast. There's probably a better way & place to fix this problem, but that would require a more in-depth look into the component. Hopefully I'll have the time to take that look sometime later, as I like the component and it would be nice to help it evolve. As a temporary fix for someone having the same problem this might work.

Add this utility function (from http://stackoverflow.com/questions/2234979/how-to-check-in-javascript-if-one-element-is-a-child-of-another )

function isDescendant(parent, child) {
    var node = child.parentNode;
    while (node != null) {
        if (node == parent) {
            return true;
         }
         node = node.parentNode;
    }
    return false;
}

and changing this function

function ins(parent, child1, child2) {
   if(child2 && !child2.parentNode) ins(parent, child2);
   parent.insertBefore(child1, child2||null);
   return parent;
}

to

function ins(parent, child1, child2) {
  if (child2) {
    if (!child2.parentNode) {
        ins(parent, child2);
    } else if (child2.parentNode && !isDescendant(parent, child2)){
        child2 = null;
    }    
  }
  parent.insertBefore(child1, child2||null);        
  return parent;
}

Opacity seems to get weird values sometimes. Hardcoding the opacity seems to fix the problem, but obviously is not a great permanent solution...

createEl('fill', {color: o.color, opacity: o.opacity}),

to

createEl('fill', {color: o.color, opacity: 0.25}),

@beardChamp
Copy link

@marqsm I was seeing exactly the same issues with the same mysterious onset after past successful passes through QA. I applied your code and was happy to see spinners again. Thanks for posting your solutions. You just saved me hours!

@aterris
Copy link

aterris commented Jan 6, 2012

I just wanted to chime in and say that I am also having this issue on IE8 (havent tested any other IE's)

For my purposes, I have simply removed spin.js and will have to move forward without using it so I don't have any additional solutions, but still wanted to mention that I am seeing that issue as well

@jasonwilliams
Copy link

Although a great idea, Im guessing this plugin wasn't properly tested with IE.

I can say though that marqsm's fix works for me for now. I'm not entirely sure what the problem is with this but someone should look at it and maybe give it high priority as without IE support its going to be pretty pointless to use on any production site.

My 2 cents

@kossnocorp
Copy link

@marqsm 👍

@techouse
Copy link

Yea... As far as IE8 and lower go, spin.js just keeps popping out JS errors. :/ None of the fixes mentioned above worked for me.

@nzbluefish
Copy link

Experience this problem with my current project. This fixed worked for me.
Cheers, Innes

@fgnass
Copy link
Owner

fgnass commented Jan 30, 2012

May I kindly ask you to post a link to a non-working example, preferably on jsfiddle? The spin.js homepage works perfectly for me in IE8 and below, so it would be interesting to find out under which circumstances the issue appears.

@techouse
Copy link

techouse commented Feb 3, 2012

Sorry, totally forgot about this... Thing is i already fixed the thing to using images and updating them every 100 miliseconds (since animateed gifs and IE don't go hand in hand) and the site is in production so I can't really use that site as an example :/ I was using it together with jQuery 1.7.1 and the jQuery plugin found here: https://gist.github.com/1290439 Maybe that's of some help to you. Maybe the jQuery plugin is broken for IE8 and down... dunno

@aterris
Copy link

aterris commented Feb 3, 2012

@techouse mentioned that he was using the jQuery plugin. I am no longer using it so I also don't have a broken example, but we were seeing the issue while using the standalone library not the jQuery plugin, so that does not seem to be the differentiation

@nzbluefish
Copy link

I will see if I can replicate it. I applied the fix as suggested and it sorted it out for me, but I can recall where I got the original error. I'll back out the fix to see if I can repeat it.

@nzbluefish
Copy link

Managed to repeat the issue by reverting back to the original spin.min.js I was using. I am running it on an IE8 (8.0.7601.17154) version. Error occurs at: Line 2, Character 849 - Invalid Argument. Let me know what I can provide to help figure this out. The website is currently in preproduction. Cheers, Innes.

@ProtectedVoid
Copy link

I am also having this issue in IE8, or, more specifically, IE9 using the Developer Tools to force IE8 standards mode. Our application is an HTML5 application, but because we have to support IE8, we are using HTML5shiv.js to add HTML5 capability to IE8.

For me, if I remove HTML5shiv.js or use the spin.min.js on a page that does not contain HTML5shiv.js, the problem goes away, and the spinners work as expected. IE9 is unaffected...it works regardless of whether HTML5shiv.js is included on the page or not.

Hopefully, that will help you some. I'd love to post a working (/ broken) example for you, but I'm running development locally.

fgnass added a commit that referenced this issue Feb 10, 2012
@jasonwilliams
Copy link

Sorry I couldnt have been help with the spinner our site is behind a login
wall so its not possible for me to show you and I wouldnt know how to
re-create the issue either.

Its screwing ie8 and ie7 about pretty bad so (using jQuery) I've disabled
it for those browsers until this is sorted.

Jason

@fgnass
Copy link
Owner

fgnass commented Feb 10, 2012

@Jayflux Have you tried the new version I pushed earlier today?

@jasonwilliams
Copy link

Yeah I tryed it an hour ago

Ie 8 was still giving me errors in the console.
I will try it again later on at home and give you some feedback

@camilonova
Copy link

@Jayflux did you test on your home?

@jasonwilliams
Copy link

Sorry not had chance to check

I will reply when i do though

j

@vesan
Copy link

vesan commented Mar 9, 2012

I tried the new version and it fixed my problem. From my point of view this ticket can be marked fixed.

@neardark
Copy link

The latest version fixed this for me as well.

@fgnass fgnass closed this as completed Mar 19, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests