Updates s9e/text-formatter to 2.x

[tankerkiller125]

Fixes #1979

Changes proposed in this pull request:

Updates s9e/text-formatter to the latest version, no changes other than composer.json were needed, tested all functionality still works and extensions such as fof/formatting still work as intended.

Reviewers should focus on:

• Should this be in the beta.12 release or a following update
• Any breaking changes I might have missed (tested emoji, fof/formatting, mentions, markdown and bbcode all working)
• Please note that this change adds new markdown support (notably spoilers) when markdown extension enabled.

Confirmed

• Frontend changes: tested on a local Flarum installation.
• Backend changes: tests are green (run composer test).

[franzliedke]

Thank you, sir!

[luceos]

While deploying to discuss we encountered an issue in firefox. Possibly xslt is not supported in this browser, did that version update to 2 perhaps? We might need a polyfill for that.

[tankerkiller125]

The following link is a side by side comparison of the render.js of s9e/text-formatter, on the left is the old one and on the right is the new one. https://gist.github.com/tankerkiller125/041361820156f76e0426bdf1e000429c/revisions I'm combing through but I've been unable to find where/why it might be erroring.

[askvortsov1]

Indeed, XSLT 2.0 isn't supported natively by firefox: https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XSLT_2.0

Maybe we could add in the Saxon JS runtime as a workaround? http://www.saxonica.com/saxon-js/index.xml

[luceos]

Can either of you do a test with saxon as a solution? We can check whether that's wanted or not in the PR.

[askvortsov1]

I can do so later today and report back. Regardless, might be good to file an issue with s2e: if this breaks Firefox for us, it'll break it for pretty much everyone. I'll also look into xstl compat with other browsers

[askvortsov1]

After some experimentation, I'm not entirely sure that xlst2 is the issue (it doesn't look like s9e/text-formatter changed to it?) I'm running a local installation of beta 12 with only bundled extensions installed, and everything works fine for me on Firefox. @luceos what version of firefox were you able to reproduce this with? I'll try running with that, as well as installing all the Discuss extensions and seeing whether that changes anything.

[tankerkiller125]

@askvortsov1 I also see the issue on Firefox 73.0.1 you can see it on https://nightly.flarum.site possibly.

[luceos]

72.0.2 (64-bit), just upgraded to 73.0.1 and having the same issue on nightly

[tankerkiller125]

It looks like it may be related to sentry?

But I'm not sure, this may just be because it's erroring before sentry initializes.

[askvortsov1]

I'm running the same firefox, and it's working fine on the bare-bones local install. Can we try disabling sentry/other extensions on nightly?

On a brief tangent, I know that if I restart my docker container running flarum and fof/sentry can't connect to sentry, the whole site crashes. While that particular issue might be specific to my use of Docker, I wouldn't be surprised if some issue in sentry could be causing this.

Daniel, were you able to confirm on a local bare-bones installation, or just on discuss and nightly?

[luceos]

We have protection in place to prevent the removal of sentry on the demo sites. 🤔

[askvortsov1]

the odd thing is, the first error log in Matt's screenshot (the one's that just a colon) originates in s9e's library. Possibly the error is handled somewhere else in s9e, but sentry tries to report it and fails, and that blocks rendering of the buttons?

Unfortunately, ":" is not a very informative error message...

[tankerkiller125]

Just tested without sentry, on Firefox 73.0.1, complete error, no tags list, we may need to revert until this issue gets fixed by the text-formatter lib.

[luceos]

@JoshyPHP is this something you encountered before? https://nightly.flarum.site with firefox. Seems somewhere this browser is not supported.

[JoshyPHP]

No, I've never heard of an issue related to a single colon. I read the comments but I haven't seen anything about the actual error. Is there an error message somewhere?

The templates use the same XSLT 1.0 as before.

[tankerkiller125]

@JoshyPHP We're just seeing the colon error, nothing more. Upon loading in Chrome I'm able to access the s9e.TextFormatter global functions but on Firefox I can not. Even with webpack dev mode I'm unable to find the exact location of the error with the JS maps.

Were there any major changes to the JS I missed when upgrading from what would have been 1.X to 2.X?

[JoshyPHP]

I looked into it and the error message is caused by Firefox's XPath lexer which has difficulties parsing some expressions.

As a temporary workaround, you can manually alter the template used for [list] with this:

$configurator->tags['list']->template = str_replace('or', '+', $configurator->tags['list']->template);

[askvortsov1]

This seems to be a pervasive issue throughout the site (for example, this also breaks the reply/like links at the bottom of each post). I'm not sure that it would be feasible to manually patch around every instance, especially when some of them might be in 3rd party extensions

Ignore that, I misunderstood the proposed solution. Thanks a lot for looking into this!

[tankerkiller125]

@askvortsov1 patching the BBCode extension resolved all issues in my dev environment.

@luceos I've created a patch to the BBCode extension flarum/bbcode#9

[JoshyPHP]

@tankerkiller125 Don't patch Flarum's extension, I'll fix the issue in my library directly and release 2.3.5 2.3.6 later today.

[tankerkiller125]

@JoshyPHP even better :)

[tankerkiller125]

I've just tested with the latest version of text-formatter and everything is working as expected now on Firefox, thanks @JoshyPHP

[luceos]

@JoshyPHP thanks for hopping into this so fast 🤗

@tankerkiller125 feel free to PR the minimum constraint for 2.3.6 if you want.

Great job!