twbs / ratchet

Build mobile apps with simple HTML, CSS, and JavaScript components.
http://goratchet.com
MIT License
14.62k stars 1.45k forks source link

IE 8 JS errors #309

Closed XhmikosR closed 10 years ago

XhmikosR commented 10 years ago
SCRIPT438: Object doesn't support property or method 'addEventListener' 
ratchet.js, line 29 character 3

I sort of worked around this but I don't like my way

  if (!window.addEventListener) {
    window.attachEvent('touchend', function (event) {
      var modal = getModal(event);
      if (modal) {
        if (modal && modal.classList.contains('modal')) modal.classList.toggle('active');
        event.preventDefault(); // prevents rewriting url (apps can still use hash values in url)
      }
    });
  } else {
    window.addEventListener('touchend', function (event) {
      var modal = getModal(event);
      if (modal) {
        if (modal && modal.classList.contains('modal')) modal.classList.toggle('active');
        event.preventDefault(); // prevents rewriting url (apps can still use hash values in url)
      }
    });
  }
mdo commented 10 years ago

Don't think this needs to support IE8 in the slightest, unless you mean IE on Windows Phone 8, in which case yeah maybe in the future per #263.

XhmikosR commented 10 years ago

I was just testing the docs site with various browsers and noticed this.

zlatanvasovic commented 10 years ago

Docs site should support all browsers like the Bootstrap, including IE 8.

@XhmikosR Did you try this? It's much cleaner.

window.addEventListener = window.addEventListener || window.attachEvent;
window.addEventListener('touchend', function (event) {
  var modal = getModal(event);
  if (modal) {
    // There is a double `modal` check, nuke it
    if (modal.classList.contains('modal')) modal.classList.toggle('active');
    event.preventDefault(); // prevents rewriting url (apps can still use hash values in url)
  }
});
XhmikosR commented 10 years ago

Unfortunately, I cannot really test anything due to those TypeErrors. IE has the bad habit of stopping the page parsing when it hits an error in JS. I'll get back to this as soon as the TypeErrors are fixed.

zlatanvasovic commented 10 years ago

@connor We should add some kind of device check. .js-device isn't enough.

mdo commented 10 years ago

Docs site should support all browsers like the Bootstrap, including IE 8.

I disagree. If you're building Android or iOS apps, or even just prototyping for them, chances are you're not using any version of IE. IE9+ sounds about right to me.

Unless these errors crash the browser, I say ignore them.

mdo commented 10 years ago

From Google Analytics:

Less than 3% of views yesterday came from IE, and only 20% of that from IE8 or lower.

So, if the error doesn't prevent page load, let's close this.

XhmikosR commented 10 years ago

I will test again tomorrow for ie9.

XhmikosR commented 10 years ago

No need to hurry this :) with the time difference I need to be a vampire :p

zlatanvasovic commented 10 years ago

@mdo 3 / (100 / 20) percents is a very small amount, just 0.6%. However, that rate may grow up with Windows Phone Ratchet support.

@XhmikosR lol

XhmikosR commented 10 years ago

Unfortunately, even IE9 is affected by what I describe above; the page isn't being parsed due to the TypeErrors. So we'll need to get those fixed and revisit this.

2014-02-27_06-54-31

zlatanvasovic commented 10 years ago

Could you update the issue to reflect that?

XhmikosR commented 10 years ago

There's nothing to update; the errors on IE9 aren't the same but as you can see from the screenshot I can't really check unless the TypeErrors are fixed.

zlatanvasovic commented 10 years ago

Okay.

2014-02-27 10:52 GMT+01:00 XhmikosR notifications@github.com:

There's nothing to update; the errors on IE9 aren't the same but as you can see from the screenshot I can't really check unless the TypeErrors are fixed.

— Reply to this email directly or view it on GitHubhttps://github.com/twbs/ratchet/issues/309#issuecomment-36226260 .

Zlatan Vasović - ZDroid

coliff commented 10 years ago

The background not displaying in IE8 and IE9 issue is because it needs the IE gradient specified:

filter: progid:DXImageTransform.Microsoft.gradient( startColorstr='#0a1855', endColorstr='#da0024',GradientType=0 );

XhmikosR commented 10 years ago

Unless the type errors are fixed nothing can be done since ie doesn't parse any code after it hits a js error.

XhmikosR commented 10 years ago

Closing since IE8 is not supported at this point.