Page MenuHomePhabricator

CodeMirror breaks on some large RTL pages
Open, LowPublicBUG REPORT

Description

Steps to replicate the issue (include links if applicable):

What happens?:

Eventually everything is either highlighted as green (same color tags are supposed to have), or purple (templates). Or, you may see no highlighting at all.

With bidi isolation applied (T358804), CodeMirror may error out entirely:

Uncaught TypeError: Cannot read properties of undefined (reading 'length')

What should have happened instead?:

Syntax highlighting should work as expected.

Other information:

This is a rather extreme example, being one of the largest articles on Arabic Wikipedia. Maybe things breaking is expected here.

What we should prevent however are the errors that can happen. Perhaps automatically disable CodeMirror due to performance reasons and show a message stating such, or something?

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Replacing all content with more RTL content (LTR seems fine for some reason?) puts CodeMirror in a broken state, and might even produce errors

I have confirmed T358804 to be the apparent cause of the errors. With older commits, the syntax highlighting is still broken, but it doesn't error out.

I'll investigate further and return here with my findings and/or a patch. I had a feeling bidi isolation could cause some problems, which is why I was planning on putting behind a preference when we tackle T359498. I think it's fine if it doesn't work occasionally, but it should never error out and prevent editing.

Seems there may a direct correlation of article size to performance and potential for errors. From cursory testing on Arabic Wikipedia's largest articles:

I do see the warning "Measure loop restarted more than 5 times" when scrolling quickly, but as I understand this is somewhat expected, and doesn't seem to be causing any real problems.

Perhaps the highlighting being broken is okay for the extreme cases? I think my goal may be only to prevent the errors, which might be as simple as a try/catch block around the bidi isolation code.