[Markdown] Toolbar causes issues in IE 11

[Ralkage]

Bug Report

Current Behavior
In Internet Explorer Version 11 (11.407.17134.0) on Windows 10 (10.0.17134 Build 17134), I am unable to create a discussion or even reply to one when I am logged in. What makes this absolutely weird is that I'm getting no errors in the console and the only reason how I came across these bugs was that I was in the process of retesting to #1174. I can reproduce this both locally and on Flarum Discuss (despite Flarum Discuss running on dev-master). I have kept in mind that Microsoft does not support this browser anymore as of January 12, 2016, but it is still bundled with the OS and is still widely used with legacy web applications.

Steps to Reproduce

Part 1:

1. Open up Internet Explorer (Version 11).
2. Go to your locally hosted Flarum or https://discuss.flarum.org/
3. Log in if you aren't already.
4. Click on the "Start a Discussion button".
5. Observe as the discussion composer does not appear.

Part 2:

1. Open up Internet Explorer (Version 11).
2. Go to your locally hosted Flarum or https://discuss.flarum.org/
3. Log in if you aren't already.
4. Open up any discussion that is not locked.
5. Attempt to reply to the discussion.
6. Observe as the reply/pos composer does not appear.

Expected Behavior
In using IE11, I should be able to create a new discussion and see the discussion composer pop-up as well as being able to reply to any discussion that is not locked.

Screenshots
N/A

Environment

• Flarum version: v0.1.0-beta.8.1 (locally hosted) or dev-master on Flarum Discuss
• Website URL: Localhost or https://discuss.flarum.org/
• Webserver: Apache (locally hosted)
• Hosting environment: Locally hosted (WAMP)
• PHP version: 7.2.4 (locally hosted)
• Browser: Internet Explorer Version 11 (11.407.17134.0)

Output of "php flarum info", run this in terminal in your Flarum directory.

Flarum core 0.1.0-beta.8.1
PHP version: 7.2.4
Loaded extensions: Core, bcmath, calendar, ctype, date, filter, hash, iconv, jso
n, mcrypt, SPL, pcre, readline, Reflection, session, standard, mysqlnd, tokenize
r, zip, zlib, libxml, dom, PDO, bz2, SimpleXML, xml, wddx, xmlreader, xmlwriter,
 openssl, curl, fileinfo, gd, gettext, gmp, intl, imap, ldap, mbstring, exif, my
sqli, Phar, pdo_mysql, pdo_sqlite, soap, sockets, sqlite3, xmlrpc, xsl
+----------------------+-----------------+--------+
| Flarum Extensions    |                 |        |
+----------------------+-----------------+--------+
| ID                   | Version         | Commit |
+----------------------+-----------------+--------+
| flarum-approval      | v0.1.0-beta.8   |        |
| flarum-bbcode        | v0.1.0-beta.8   |        |
| flarum-emoji         | v0.1.0-beta.8   |        |
| flarum-lang-english  | v0.1.0-beta.8   |        |
| flarum-flags         | v0.1.0-beta.8.1 |        |
| flarum-likes         | v0.1.0-beta.8.1 |        |
| flarum-lock          | v0.1.0-beta.8   |        |
| flarum-markdown      | v0.1.0-beta.8   |        |
| flarum-mentions      | v0.1.0-beta.8.1 |        |
| flarum-statistics    | v0.1.0-beta.8   |        |
| flarum-sticky        | v0.1.0-beta.8   |        |
| flarum-subscriptions | v0.1.0-beta.8   |        |
| flarum-suspend       | v0.1.0-beta.8   |        |
| flarum-tags          | v0.1.0-beta.8.2 |        |
+----------------------+-----------------+--------+
Base URL: http://b81.test
Installation path: C:\wamp64\www\b81.test
Debug mode: off

Possible Solution
N/A

Additional Context
N/A

[PeopleInside]

Thank you for this report. I am able to reproduce the same issue on Beta 8.1 and IE 11

[MichaelBelgium]

Not sure if you guys already know but it does generate a javascript error

SCRIPT5009: 'Reflect' is undefined
forum-cf6a6fcc.js (530,14336)
SCRIPT5009: 'Reflect' is undefined
forum-cf6a6fcc.js (530,14336)

And after nothing really works anymore except for refreshing the page

[dsevillamartin]

@MichaelBelgium Hm, flarum's source code has no mention of any Reflect class, it could be one of the dependencies and/or extensions. Could you perhaps reproduce this locally with debug mode enabled and post the code surrounding the line erroring?

[PeopleInside]

I hope this can be solved soon. Sorry i cannot help but seems version Beta 8 was not having this issue.
https://forum.fibra.click/ has not this issue has is not updated to Flarum 8.1 :)

BUT.. maybe is Internet Explorer who doesn't work very well. In the past i found issues on Edge where on my blog some link image was not clickable. Now the issue has been resolved in Edge but is present on Internet Explorer... so maybe Flarum is not working on Internet explorer because of browser issue also... in any case seems version 8.0 (Flarum) was working.

[franzliedke]

@datitisev There is a call to Reflect.construct in the compiled JS code. Must be an artifact of JS transpilation - and apparently one without a fallback for IE11.

[dsevillamartin]

@franzliedke Internet explorer seems to be the only major browser that does not support it... do we want to include (see the browser compatibility table).
Do we want to add a polyfill (i.e. https://github.com/tvcutsem/harmony-reflect) simply for IE?

[franzliedke]

Basically, yes.

I suppose we should make proper use of @babel/preset-env (which is already part of our webpack config) to properly target a specific set of browsers, instead of handpicking polyfills whenever we encounter issues.

But I have far too little experience with Webpack and Babel to a) make this change and b) judge whether the current setup has other benefits.

[franzliedke]

I believe this is due to one of the extensions.

@Ralkage Can I ask you to disable extensions and enable them one-by-one to find out which one it is? Alternatively, someone could dig through the compiled JS in all the extensions mentioned above. 😉

[PeopleInside]

@franzliedke Hi :)
I disabled all external extensions and the issue with IE 11 persist.

Is just mine test, feel free to test more.

Seems not to me to be an extension if is not a core extension to give issues.

[franzliedke]

I meant all of them, including the core extensions.

I identified the faulty one: disable flarum/markdown, and the problem should be fixed. Probably a problem with the custom-elements polyfill that we need for GitHub's markdown toolbar.

[PeopleInside]

I meant all of them, including the core extensions.

Sorry :)

I identified the faulty one: disable flarum/markdown, and the problem should be fixed. Probably a problem with the custom-elements polyfill that we need for GitHub's markdown toolbar.

Confirmed 👍

[franzliedke]

The fix is rolled out on discuss - please confirm!

[PeopleInside]

It works for me. Thanks.

[franzliedke]

Perfect.

And thanks @datitisev !!!

[PeopleInside]

@franzliedke in my test works mean now i am able to create post on IE but... i am trying to insert a link. Seems button is visible but code is not working. It's only me?

Seems the markdown bar is visible but commands doesn't works, works for you?

[franzliedke]

Oh yes, the toolbar is broken even in a new Chrome browser now. 😕

[dsevillamartin]

Hm, seems like the pollyfill I PR'd changed the functionality of HTMLElement. 🤦‍♂️ I don't know how I didn't notice.

[franzliedke]

Custom elements are a pain to bundle for a large range of browsers. I am trying to integrate babel-plugin-transform-custom-element-classes, but not having success so far.

[dsevillamartin]

We could disable the markdown bar on IE. Simple and easy.

[franzliedke]

That would be fine as well.

[i4xB0y]

I think this should be an absolute last resort

[tobyzerner]

Relevant for understanding: webcomponents/custom-elements#90

Ultimately our goal will be to stop using github-markdown-toolbar, and just extract the useful parts (mainly the textarea manipulation logic) without any of the custom element stuff. Not sure if we can get this done for beta 9? I'd say time is better spent doing that than it is trying to change up our babel/webpack config.

[luceos]

I'm proposing to go for the proposal by @datitisev ; drop the toolbar in case the user is on IE. That browser is no longer supported anyway and users can still type manually.

@flarum/core please Yea/Nay so we can get b9 out.

For beta 10 or 11 we will look at implementing our own toolbar or find a suitable replacement.

[tobyzerner]

Fine with me!

[franzliedke]

What about conditionally loading the toolbar only if the browser natively supports custom elements? That would solve the IE issue, plus reduce file size...

[tobyzerner]

We just need the simplest lowest-effort stopgap solution for now, any further work should be put into the definitive solution (replacing github-markdown-toolbar). So rather than fiddling around with multiple JS files and conditional loading, I'd suggest just use a conditional feature-detection at runtime and don't initiate the toolbar if Reflect API is not present?

[luceos]

I agree with @tobscure, get beta 9 out first. Design a better implementation for the next release. Can you agree on that @franzliedke ?

[franzliedke]

The most important issue for me is doing feature detection and not browser detection. Whether checking for Reflect.construct is enough, I don't know.

[dsevillamartin]

Did just this and made a PR at flarum/markdown#7.

Wasn't sure if require as available in Webpack, apparently it is. I just checked for window.Reflect before requiring those modules, and added a check in the initializer to return if the MarkdownArea variable hasn't been initialized.

...And yes, the buttons work now 🤦‍♂️.

[luceos]

Do we want to re-use this issue for the toolbar refactoring or create a new one?

[ivangretsky]

Hey! I just tried to create a new discussion and post to an old one in IE 11 and it worked:
https://discuss.flarum.org/d/19673-ie-11-test
Could anybody try to check and confirm?

[PeopleInside]

@ivangretsky at the moment the Flarum editor on the official forum is broken so this is why you see you are able to post on Internet Explorer 11.

Try to put a link with the editor and you will see you are not able to do. We need wait for the fix... is working in progress.

[ivangretsky]

Was not that easy (((

[PeopleInside]

The editor on the official Flarum community continue to be broken.

[luceos]

@PeopleInside that's because discuss hasn't been updated yet. Have some patience please.

[PeopleInside]

@luceos no problem for me. Just something to inform you, i was suppose maybe you forget or don't' know.
Have a good day :)

[al3xLvs]

I think this issue shouldn't block a release. Microsoft does not recommend IE 11 by default.

https://techcommunity.microsoft.com/t5/Windows-IT-Pro-Blog/The-perils-of-using-Internet-Explorer-as-your-default-browser/ba-p/331732

[dsevillamartin]

@Magicalex This issue has been resolved in the latest version of flarum/markdown. At least, not being able to reply. IE support for the markdown bar isn't yet available, but that won't be added in beta 9.

[aguilaair]

So, from what I understand Beta 9 is ready?

[dsevillamartin]

@aguilaair Beta 9 only disables the markdown toolbar on IE, it still doesn't support it, so that's why this issue remains open.