Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot Create or Reply to Discussions Using IE11 #1702

Open
Ralkage opened this issue Dec 14, 2018 · 40 comments

Comments

Projects
None yet
@Ralkage
Copy link

commented Dec 14, 2018

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

This comment has been minimized.

Copy link

commented Dec 15, 2018

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

@MichaelBelgium

This comment has been minimized.

Copy link

commented Jan 6, 2019

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

@datitisev

This comment has been minimized.

Copy link
Member

commented Jan 6, 2019

@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

This comment has been minimized.

Copy link

commented Jan 26, 2019

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

This comment has been minimized.

Copy link
Member

commented Feb 3, 2019

@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.

@datitisev

This comment has been minimized.

Copy link
Member

commented Feb 3, 2019

@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

This comment has been minimized.

Copy link
Member

commented Feb 3, 2019

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

This comment has been minimized.

Copy link
Member

commented Feb 5, 2019

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

This comment has been minimized.

Copy link

commented Feb 5, 2019

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

Is just mine test, feel free to test more.
00

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

@franzliedke

This comment has been minimized.

Copy link
Member

commented Feb 5, 2019

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

This comment has been minimized.

Copy link

commented Feb 5, 2019

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

This comment has been minimized.

Copy link
Member

commented Mar 20, 2019

The fix is rolled out on discuss - please confirm!

@PeopleInside

This comment has been minimized.

Copy link

commented Mar 20, 2019

It works for me. Thanks.

@franzliedke

This comment has been minimized.

Copy link
Member

commented Mar 20, 2019

Perfect.

And thanks @datitisev !!!

@PeopleInside

This comment has been minimized.

Copy link

commented Mar 20, 2019

@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

This comment has been minimized.

Copy link
Member

commented Mar 20, 2019

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

@franzliedke franzliedke reopened this Mar 20, 2019

@datitisev

This comment has been minimized.

Copy link
Member

commented Mar 20, 2019

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

image

@franzliedke

This comment has been minimized.

Copy link
Member

commented Mar 20, 2019

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.

@datitisev

This comment has been minimized.

Copy link
Member

commented Mar 20, 2019

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

@franzliedke

This comment has been minimized.

Copy link
Member

commented Mar 20, 2019

That would be fine as well.

@i4xB0y

This comment has been minimized.

Copy link

commented Mar 21, 2019

I think this should be an absolute last resort

@tobyzerner

This comment has been minimized.

Copy link
Member

commented Mar 22, 2019

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

This comment has been minimized.

Copy link
Member

commented Apr 9, 2019

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

This comment has been minimized.

Copy link
Member

commented Apr 9, 2019

Fine with me!

@franzliedke

This comment has been minimized.

Copy link
Member

commented Apr 9, 2019

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

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

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

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

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

@franzliedke

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

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.

@datitisev

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

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

This comment has been minimized.

Copy link
Member

commented Apr 11, 2019

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

@ivangretsky

This comment has been minimized.

Copy link

commented Apr 11, 2019

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

This comment has been minimized.

Copy link

commented Apr 11, 2019

@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

This comment has been minimized.

Copy link

commented Apr 11, 2019

Was not that easy (((

@PeopleInside

This comment has been minimized.

Copy link

commented Apr 17, 2019

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

@luceos

This comment has been minimized.

Copy link
Member

commented Apr 17, 2019

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

@PeopleInside

This comment has been minimized.

Copy link

commented Apr 17, 2019

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

@Magicalex

This comment has been minimized.

Copy link

commented Apr 20, 2019

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

@datitisev

This comment has been minimized.

Copy link
Member

commented Apr 20, 2019

@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

This comment has been minimized.

Copy link

commented May 23, 2019

So, from what I understand Beta 9 is ready?

@datitisev

This comment has been minimized.

Copy link
Member

commented May 23, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.