activeElement can be null

[mAAdhaTTah]

I'm sometimes getting an error on from this line while testing in IE11. Apparently, activeElement can be null, so the line throws an error. I don't think it materially affects the script, but maybe an existence check there would be an improvement? If I get around to it, I can open a PR, but figure I should flag you on it.

[ten1seven]

Ah, cool. Thanks!

[localpcguy]

I see the same error on this piece of code (which appears to be the same line listed above)

if (currentInputType !== newInputType && ignoreMap.indexOf(eventKey) === -1 && formInputs.indexOf(document.activeElement.nodeName.toLowerCase()) === -1) {

[ten1seven]

Hi @mAAdhaTTah and @localpcguy,

I've addressed this as part of a v4.0 update. Would you be up for checking it out and letting me know if this resolves the bug you're seeing?

I'd recommend looking over the updated README to see what's changed.

https://github.com/ten1seven/what-input/blob/version-4/src/what-input.js#L128

[mAAdhaTTah]

var activeInput = (
  formInputs.indexOf(document.activeElement.nodeName.toLowerCase()) === -1
) ? true : false;

This still won't work because document.activeElement can be null, so attempting to access nodeName will throw. Something like:

var activeInput = (
  document.activeElement && formInputs.indexOf(document.activeElement.nodeName.toLowerCase()) === -1
) ? true : false;

should be fine.

[ten1seven]

Ah, okay thanks. Do you have steps to reproduce the bug so I can test?

[mAAdhaTTah]

Honestly, it happened kind of randomly. I'm not really sure how to reproduce it consistently. Sorry.

[ten1seven]

This fix has been released in v4.x. Give it a shot when you have a chance.

[localpcguy]

I'll see if I can test it, I think we have already handed off that project with a manual fix to production, but I probably still have the code locally.

[priithansen]

Seems that we are having related errors with IE11 users and somehow activeElement seems to be truthy with nodeName still undefined

[ten1seven]

@priithansen thanks for these details! I'm checking it out.

[ten1seven]

@priithansen I'm still not able to reproduce this error in IE11. If you have any steps or anything happening on that page that I could try it would be helpful.

Alternately, I took a stab and adding an extra check to make sure nodeName exists. Can you check out the version in this branch and let me know if the error persists?

https://github.com/ten1seven/what-input/tree/jf/activeelement-ie11-fix

[priithansen]

Thanks for the quick fix. I'm not able to reproduce it either so it looks like the easiest way to check is to but the fix live on our site and see if the sentry errors start to go down.

I will report back probably some time next week when we have collected some statistics.

[priithansen]

Happy to report that we have not had any errors from IE11 after putting the ie11-fix live (used to be tens to hundreds a day). Not sure if the problem is now just buried deeper or fixed :)

[ten1seven]

👍 thanks for the update! I added an extra check for activeElement so hopefully it resolves the issue. Happy to hear back if you report any more issues. I'll merge this into master and release it as v4.0.5 this morning.

[EasterPeanut]

This issue is introduced again after this commit: f7f5338add2986d3a8c8ab4fde16d3facdb7f8bd

if (activeElem.nodeName.toLowerCase() === 'button' && !activeElem.closest('form')) {
  ...
}

Edit: seems like in IE11 the wrong element is returned by document.activeElement when using display: flex;:

• https://stackoverflow.com/questions/30935637/wrong-activeelement-in-focus-in-ie11
• https://github.com/Fyrd/caniuse/issues/1682

[ten1seven]

🤦‍♂ thanks for re-discovering this @EasterPeanut! Before I get testing, you're only seeing this in IE11, correct?

[EasterPeanut]

🤦‍♂ thanks for re-discovering this @EasterPeanut! Before I get testing, you're only seeing this in IE11, correct?

Yes, well I have not reproduced it myself, but in our application all users that encountered this error were using IE11.

[ten1seven]

@EasterPeanut I'm, unfortunately, as before, unable to reproduce this issue with my virtual machines running IE 11.0.9600.18860 on Windows 7 and 11.0.9600.18861 on Windows 8.1. Do you have any other details about what's triggering the issue that I can use for testing?

This is the test code I'm using pretty much straight from the stackoverflow article:

<div id="d" tabindex="1">
  <div>
    <span>span</span>
  </div>
</div>

#d {
  display: flex;
}

span {
  display: -ms-flexbox;
  display: flex;
}

document.getElementById('d').addEventListener('click', function() {
  console.log('active element is ', document.activeElement.nodeName);
});

[EasterPeanut]

@ten1seven I do get different results from clicking that span in:

• Opera 60.0.3255.95 on MacOS Mojave 10.14.4 -> DivElement
• IE11 11.503.17763.0 on Windows 10 Pro N version 1809 -> SpanElement

Where it fails in our app:

[JiaPingWu]

Ha 在 2019年5月28日 星期二 上午8:40:38 [UTC]， Piennotifications@github.com 寫道：

@ten1seven I do get different results from clicking that span in:

• Opera 60.0.3255.95 on MacOS Mojave 10.14.4 -> DivElement
• IE11 11.503.17763.0 on Windows 10 Pro N version 1809 -> SpanElement

Where it fails in our app:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[ten1seven]

Well, I'm getting closer. I can reproduce the issue but can't get an error out of what-input. Can you please send me (if you're allowed to) the markup and CSS snippets for the ._readmore_9a791__controls component so I can test against exactly what you're getting the error on?

Also, the snippet you referenced is in version 5.2.0 or earlier. Version 5.2.1 has some refactoring, including a check against activeElement being null: https://github.com/ten1seven/what-input/blob/v5.2.1/src/scripts/what-input.js#L214-L219 Would you be up for dropping that into your code and testing?

[EasterPeanut]

Also, the snippet you referenced is in version 5.2.0 or earlier.

True, and we use the later version, but the problem persists there. After the || the following is not checked anymore: activeElem && activeElem.nodeName.

Before the OR:

        (activeElem && activeElem.nodeName && formInputs.indexOf(activeElem.nodeName.toLowerCase()) === -1)

After the OR:

        (activeElem.nodeName.toLowerCase() === 'button' && !checkClosest(activeElem, 'form'))

Our code, tried to simplify it as much as possible:

<div class="ReadmoreCell">
  <div id="readmore-6364e8d4-1b0e-4345-af38-80f3b27e5bfe" class="content">
    <div class="RichContentCell">
      <p>Text</p>
    </div>
  </div>

  <div class="controls">
    <button class="toggle-button" aria-expanded="false" aria-controls="readmore-6364e8d4-1b0e-4345-af38-80f3b27e5bfe">
      <span class="more-label">Read more</span>
      <span class="less-label">Show less</span>
    </button>
  </div>
</div>

.ReadmoreCell {
  position: relative;
}

.content {
  overflow: hidden;
  position: relative;

  &::after {
    background-image: linear-gradient(
      rgba(255, 255, 255, 0),
      rgb(255, 255, 255) 90%
    );
    bottom: 0;
    content: "";
    height: 50px;
    left: 0;
    position: absolute;
    right: 0;
    z-index: 1;
  }
}

.controls {
  border-top: 1px dashed grey;
  height: 0;
  margin-top: 10px;
  position: relative;
  text-align: center;
  z-index: 2;
}

.toggle-button {
  @mixin button-base;

  background: white;
  border: 1px solid grey;
  border-radius: 100px;
  padding: 0.3em 0.6em;
  transform: translateY(-50%);

  [data-whatinput="mouse"] & {
    outline: none;
  }
}

.more-label {
  display: inline;
}

.less-label {
  display: none;
}

.show,
.disabled {
  .content {
    max-height: none !important;
    overflow: visible;

    &::after {
      display: none;
    }
  }
}

.show {
  .controls {
    border: 0;
    margin-top: 20px;
  }

  .more-label {
    display: none;
  }

  .less-label {
    display: inline;
  }
}

.disabled {
  .controls {
    display: none;
  }
}

class ReadmoreCell extends Cell {
  static defaults = {
    height: 160
  };

  initialize() {
    this.height = this.params.height || ReadmoreCell.defaults.height;

    this.toggleButton = this.element.querySelector(".toggle-button");
    this.content = this.element.querySelector(".content");
    this.controls = this.element.querySelector(".controls");

    this.toggleButton.addEventListener("click", this.toggle);

    this.checkHeight();
    window.addEventListener("resize", this.checkHeight);
  }

  toggle = event => {
    event.preventDefault();

    if (this.isExpanded()) {
      this.hide();
    } else {
      this.show();
    }
  };

  show() {
    this.element.classList.add("show");
    this.toggleButton.setAttribute("aria-expanded", true);
  }

  hide() {
    this.element.classList.remove("show");
    this.toggleButton.setAttribute("aria-expanded", false);
  }

  isExpanded() {
    return this.element.classList.contains("show");
  }

  checkHeight = () => {
    if (this.content.clientHeight >= this.height) {
      this.element.classList.remove("disabled");
      this.content.style.maxHeight = "${this.height}px";
    } else {
      this.element.classList.add("disabled");
      this.content.style.maxHeight = null;
    }
  };

  onDestroy() {
    window.removeEventListener("resize", this.checkHeight);
  }
}

Somehow I think it has got something to do with the fact we are hiding elements. Maybe IE11's activeElement is 'label-more' / 'label-less' span, while other browsers return the 'toggle-button' as activeElement. Then again, hiding an element is not removing it from the DOM..

[ten1seven]

Ah, yep yep, thanks for catching that conditional logic error! I made this update https://github.com/ten1seven/what-input/commit/a73ddbad6c497fb38a6f6f98ec7b765887d7b1c7 and created a pre-release. Would you or anyone else following this thread be able to give it a test to see if that resolves the error?

https://github.com/ten1seven/what-input/releases/tag/v5.2.2

[EasterPeanut]

I still can not reproduce the issue myself on my Windows VM + IE11 with the 5.2.1 version. But I did test the code of the pre-release and it works for me :)

[JiaPingWu]

Need permission. 在 2019年6月3日 星期一 上午9:23:00 [UTC]， Piennotifications@github.com 寫道：

I still can not reproduce the issue myself on my Windows VM + IE11 with the 5.2.1 version.

But I did test the code of the pre-release and it works for me :)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[ten1seven]

Thanks, @EasterPeanut, that fix was all you! I'm gonna give the solution a little more testing to make sure I didn't miss anything and then release it.

[ten1seven]

This fix is now in release v5.2.3