Details elements in IE and Edge

heterodox

@heterodox said in The Official Status Thread:

If I happen to find some time tomorrow, I'll see if I can't record this and put it in the Bugs forum, unless @ben_lubar or @julianlam already know about it. It's a really fucked up bug.

Let's see if this works:

Clicking details element results in a null edit?!

Don't know how much information I can provide other than that I can consistently re-create, on different computers, in both IE 11 and Edge.

heterodox

@heterodox When I try to expand details elements that aren't in my own posts, they just don't expand:

Help where are my spoilers

TheCPUWizard

Silly question, did anyone look at the debugger? [F12]

Luhmann

@thecpuwizard

heterodox

@thecpuwizard said in Details elements in IE and Edge:

Silly question, did anyone look at the debugger? [F12]

In that page of the Status thread, I see:

SCRIPT16389: Unspecified error.
File: widgets.js, Line: 11, Column: 2512

But that's a Twitter script, so it appears to be unrelated. There are no additional errors when I try to expand.

I don't even see the edits being made because they go over WebSocket. So yeah, not much to go on.

heterodox

Bump?! Not being able to expand details elements and having to click View raw is a constant annoyance.

ben_lubar

@heterodox said in Details elements in IE and Edge:

Bump?! Not being able to expand details elements and having to click View raw is a constant annoyance.

Does it work in isolation (without our weird mix of scripts)? https://fiddle.jshell.net/zspktc2d/show/

ChaosTheEternal

Edge and IE don't support <details>, though Edge may support it eventually.

anotherusername

You could try running this socket monitoring code to dump whatever's going across the websocket when you do that...

/post/1128250

Haven't tested it on IE...

Alternately, you could try setting localStorage.debug = '*'; from the Javascript console, and then see what it logs... I think that requires a refresh to take effect...

heterodox

@ben_lubar said in Details elements in IE and Edge:

Does it work in isolation (without our weird mix of scripts)? https://fiddle.jshell.net/zspktc2d/show/

I see "TestHello, World!"

@chaostheeternal said in Details elements in IE and Edge:

Edge and IE don't support <details>, though Edge may support it eventually.

It has absolutely worked on this forum in the past. Probably due to one of the scripts in our "weird mix" that has since experienced a regression. There's some sort of special handling given the blue "expand" triangle that appears on the left of the summary.

ChaosTheEternal

@heterodox said in Details elements in IE and Edge:

due to one of the scripts

Well, here is the script that should make <details> work in IE/Edge:

https://what.thedailywtf.com/assets/node_modules/nodebb-plugin-tdwtf-customizations/details-shim.js

As far as I can see, it is still working, but something else is interfering and blocking the hidden checkbox from being "checked" which is what is used to simulate the toggle to show the details. If you unhide the checkbox, you can see that you can't check it, even when clicking directly on it.

blakeyrat

@chaostheeternal said in Details elements in IE and Edge:

Edge and IE don't support
, though Edge may support it eventually.

If Edge, Microsoft Edge, pulled a feature due to "quality issues" can you even fucking IMAGINE how buggy it must have been!? I mean, look at the mess of shit they actually shipped!

Also good jerb on that post quoting NodeBB.

heterodox

@chaostheeternal said in Details elements in IE and Edge:

As far as I can see, it is still working, but something else is interfering and blocking the hidden checkbox from being "checked" which is what is used to simulate the toggle to show the details. If you unhide the checkbox, you can see that you can't check it, even when clicking directly on it.

Could be. But what the bloody hell is causing a null edit every time I try to toggle? That's the most bizarre behavior I've ever seen, either from Discourse or NodeBB.

ChaosTheEternal

It's the Markdown plugin: https://what.thedailywtf.com/assets/node_modules/nodebb-plugin-markdown/public/js/client.js

The TL;DR of the script is, basically, it binds a click event to all checkboxes in a post. In the click event, if it's not your post, it calls preventDefault() to "make them readonly" per a comment, then in either case, a "post edit" message is sent on the websocket.

I don't see any way to "fix" it short of having the Markdown plugin change, since it adds the event handler to the checkbox when the posts load, while the Details shim only runs on the initial load. Other option is to find a way to implement the Details shim without a checkbox, and hope it doesn't get trampled over in the future.

anotherusername

@chaostheeternal said in Details elements in IE and Edge:

It's the Markdown plugin: https://what.thedailywtf.com/assets/node_modules/nodebb-plugin-markdown/public/js/client.js

The TL;DR of the script is, basically, it binds a click event to all checkboxes in a post. ... a "post edit" message is sent on the websocket.

Why in the fucking hell would it do that? What reason could there possibly be to assume clicking a checkbox in a post = edit?

Is it trying to make it so that the checkbox can be checked/unchecked by the person who created the post? And in so doing, edit their post to reflect the change? Why? That makes as much sense as automatically editing the post to set <details open="true"> (or just <details>) when the post's creator opens (or closes) it.

edit: the link 404s, but this looks like the code:

nodebb-plugin-markdown/public/js/client.js at b942b0b64e291e486b33abe280e903077dca1f92 · NodeBB/nodebb-plugin-markdown

A Markdown parser for NodeBB. Contribute to NodeBB/nodebb-plugin-markdown development by creating an account on GitHub.

0_1523979696776_802e4fba-7d79-49a1-a590-bfbdb074869d-image.png

anotherusername

As far as changing it goes... I suppose using radio buttons instead might be an option. I can't really think of any other element that would automatically change state when clicked and back when clicked again, though. The alternative would be to juggle classes and use Javascript. That would be simple enough, but the checkbox/CSS version is kind of elegant in that it doesn't need to use Javascript.

There are a couple of CSS selectors that can operate in a vaguely similar manner:

:focus -- the drawback here is that only one element can be focused, and the opened container would close as soon as the element lost focus. This would make it impossible to click anything clickable (such as a link) inside the container; it'd disappear before your click registered on it.

:target -- this would be better, but since the page can only have one target (that's the portion after the hashtag in a URL), you'd still only be able to open one at a time... nested <details> wouldn't work.

ChaosTheEternal

@anotherusername said in Details elements in IE and Edge:

the link 404s

Yeah, because that's what the source map gave as the path. The actual script is merged and minified into the main nodebb.min.js file.

anotherusername

@chaostheeternal said in Details elements in IE and Edge:

Yeah, because that's what the source map gave as the path.

Which coincidentally means that source maps are broken.

edit: well, that, and the .map file 404s. Probably mainly just the .map file.

anotherusername

@julianlam could you maybe elaborate on the rationale behind the state-saving code for checkboxes?

It might be the simplest just to make the CSS that selects checkboxes more specific. Or... if you use getComputedStyle and make sure that they're not display:none;, it would exclude the hidden checkboxes that make the details/summary shim work.

For reference, the details/summary shim has the style:

.details>input:first-child {
    display: none;
}

hungrier

@chaostheeternal said in Details elements in IE and Edge:

It's the Markdown plugin: https://what.thedailywtf.com/assets/node_modules/nodebb-plugin-markdown/public/js/client.js

The TL;DR of the script is, basically, it binds a click event to all checkboxes in a post. In the click event, if it's not your post, it calls preventDefault() to "make them readonly" per a comment, then in either case, a "post edit" message is sent on the websocket.

I don't see any way to "fix" it short of having the Markdown plugin change, since it adds the event handler to the checkbox when the posts load, while the Details shim only runs on the initial load. Other option is to find a way to implement the Details shim without a checkbox, and hope it doesn't get trampled over in the future.

The WTF Bites thread is

julianlam

Checkbox state saving is present so you can have checkboxes

[ ] in your
[ ] markdown

... and clicking them updates the post. If they're interfering due to being inside the details element, that's a bug and should be fixed with a stricter CSS rule...

wow, checkboxes don't work here, that's odd.

test

Edit: I am still confused as to why a details element would interfere with my code selecting checkboxes... it doesn't seem to contain an input element

ChaosTheEternal

@julianlam said in Details elements in IE and Edge:

why a details element

In Internet Explorer or Edge, which don't natively support them, so a shim is used, which implements a checkbox and CSS to hide/show the "details" based on the checkbox being checked or not.

@julianlam said in Details elements in IE and Edge:

would interfere with my code

The checkbox added by the shim is what is being interfered with, as it can't be checked (which would toggle the details) without triggering an edit (if it's your post) and/or blocking the check from happening (if it's someone else's post).

I will note the shim is a TDWTF customization. I have a topic here that covers how to replicate it in non-IE/Edge browsers by forcing the shim to apply despite details being supported.

Tsaukpaetra

@julianlam said in Details elements in IE and Edge:

wow, checkboxes don't work here, that's odd.

0_1523993117331_4d06a097-8ca0-46a5-85ca-d0ea0fb9498b-image.png

Ben's html sanitizer doesn't allow input elements.

In any case, having posts able to be self-modifying is a surprising misfeature. Wondering if there's anything in there that can be abused...

anotherusername

Alright, I've modified my shim so it toggles a class instead of using a checkbox.

Javascript: (replaces details-shim.js)

window.addEventListener('DOMContentLoaded', function () {
  var isDetailsSupported = (function () {
    var d = document.createElement('details');
    if (!('open' in d)) {
      return false;
    }

    var p = d.appendChild(document.createElement('p'));
    p.appendChild(document.createTextNode('?'));
    document.body.appendChild(d);
    var h = p.offsetHeight;
    document.body.removeChild(d);

    return !h;
  })();

  // add a shim for <details> and <summary> if the browser doesn't support them
  if (!isDetailsSupported) {
    var r = function() {
      try {
        var d;
        while ((d = document.querySelector('details')) !== null) {
          var open = d.hasAttribute('open') || d.getAttribute('data-open') === 'open';

          var details = document.createElement('div');
          details.classList.add('details');
          if (open) details.classList.add('open');

          var summary = d.querySelector('summary');
          if (!summary) {
            summary = document.createElement('summary');
            summary.appendChild(document.createTextNode('Details'));
          }
          details.appendChild(summary);
          summary.addEventListener('click', function (e) {
            e.currentTarget.parentElement.classList.toggle('open');
          });

          var contents = document.createElement('div');
          details.appendChild(contents);
          while (d.firstChild) { contents.appendChild(d.firstChild); }

          d.parentElement.insertBefore(details, d);
          d.parentElement.removeChild(d);

          details.parentElement.normalize();
        }
      } catch (e) {
        if (window.console && window.console.error) {
          window.console.error(e);
        }
      }
    };

    new MutationObserver(r).observe(document.body, {childList: true, subtree: true});
    r();
  }
}, false);

CSS: (replaces details-shim.less)

.details, summary {
    display: list-item;
    font-weight: inherit;
}

.details > summary {
    display: inline-block;
    font-weight: inherit;
}

.details > summary:before {
    content: "►";
    display: block;
    margin-top: 1pt;
    font-size: 80%;
    width: 1.3em;
    float: left;
}

.details.open > summary:before {
    content: "▼";
    margin-left: -2px;
    margin-right: 2px;
}

.details:not(.open) > :not(summary) {
    display: none;
}

If someone would like to update it on Github, I think it should pretty much be copy and paste. I'm not as familiar with LESS syntax, so I'll let you convert my CSS if necessary.

@ben_lubar

julianlam

@tsaukpaetra said in Details elements in IE and Edge:

In any case, having posts able to be self-modifying is a surprising misfeature. Wondering if there's anything in there that can be abused...

While I'm here, I should point out that the self-modifying functionality on checkbox change is a feature in nodebb-plugin-markdown which is covered by our bug bounty.

If you can find a way to abuse it (e.g. modify someone else's post), report it to security@nodebb.org for triage and a bounty.