search

msn0 / mdn-polyfills

MDN polyfills - from, forEach, filter, find, findIndex, assign, includes, create, entries, of, repeat, startsWith, endsWith, toggleAttribute, bind, MouseEvent, CustomEvent, padEnd, padStart
https://msn0.github.io/mdn-polyfills
175 stars 25 forks source link

Element.classList.toggle #57

Open mad-bizarre opened 5 years ago

mad-bizarre commented 5 years ago

Hi, there is a bug in Element.classList.toggle.

First: https://github.com/msn0/mdn-polyfills/blob/master/src/Element.prototype.classList/classList.js#L157 - this.remove(oldToken), there is no variable oldToken in this function, this is copy-paste from next method DOMTokenListProto.replace, there must be a "val" variable from arguments.

Second: I replaced this.remove(oldToken) with this.remove(val) but this code is not work in Android 4.2 browser\webview (but it works in Chrome, for example), because expression "this.value" in Android 4.2 returns undefined, so className always added, but never removed.

I tried to submit bug into MDN documentation, but got: Sorry, entering a bug into the product Developer Documentation has been disabled.

Simple implementation in russian version of doc works, but not consider the second argument of "toggle" method https://developer.mozilla.org/ru/docs/Web/API/Element/classList

ccchapman commented 3 years ago

I am also experiencing a problem with Element.classList.toggle's polyfill not working. The second argument, force, is ignored on IE 11. It seems like the polyfill may not be applied in this situation.

This can also be reproduce by grabbing the most up-to-date polyfill snippet from MDN and running a basic example. In this case, you'd expect the element to remain red.

<style>
  .red {
    background: red;
  }
</style>

<div class="red">
  Red
</div>

<script>
  /**
   * [...]
   * Polyfill from https://developer.mozilla.org/en-US/docs/Web/API/Element/classList#polyfill
   */
  document.querySelector(".red").classList.toggle("red", true);
</script>