Page MenuHomePhabricator

Make CodeMirror support IME functionality
Open, NormalPublic5 Story Points

Description

Positioning of ULS keyboard icon

Whenever I click in the textarea on a freshly loaded page, the ULS keyboard icon pops up there or at the end of the closest line. It can change positions if I reload the page.


ULS transliteration

In Chromium browser (and Google Chrome), while trying to type in Malayalam using transliteration, Malayalam inscript or Malayalam inscript 2 keyboards available in ULS, Malayalam letters appear but revert to English on next keystroke if Wikitext syntax highlighting (CodeMirror ?) is enabled from the beta features.

In firefox, ULS keyboards work as expected but sometimes input fields freeze and no letters appear at all. But every time ULS' keyboard widget appears in an absurd position, and menu dropdown appears away from the widget (Main edit field).

Once Wikitext syntax highlighting disabled, everything goes back to normal. Please note that this happens only in 'Source editing' (ULS' inputs work pefectly in 'visual editing' mode)

Tested browsers:
Chromium Version 65.0.3325.181 (Official Build) Built on Ubuntu, running on LinuxMint Cinnamon 64 bit
Googe Chrome Version 66.0.3359.117 (Official Build) (64-bit)

Firefox 59.0.2, LinuxMint Cinnamon 64 bit

To recreate the problem:

  1. Go to user preference, enable Wikitext syntax highlighting from beta features section
  2. Go to some edit window (eg: https://ml.wikipedia.org/w/index.php?title=Test1&action=edit&redlink=1)
  3. Type using ULS in visual editor's 'Source editing' mode

Details

Related Gerrit Patches:
mediawiki/extensions/UniversalLanguageSelector : masterAdd support for CodeMirror
mediawiki/extensions/CodeMirror : masterAdd UniversalLanguageSelector support

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 22 2017, 7:13 AM

Can reproduce with Firefox 54.
Cannot manage to show that ULS icon at all in Chromium 59.

I get similar issues in Chromium. Occasionally, the ULS icon even appears offscreen (and causes a horizontal scrollbar to be shown):

matmarex renamed this task from Codemirror in Firefox shows ULS icon in wrong positions to CodeMirror shows ULS icon in wrong positions.Aug 4 2017, 1:28 PM

I see this bug all the time in Firefox and it makes it difficult to edit sometimes. @Amire80: Any suggestions? Perhaps you could point us in the direction of where the relevant code is that controls the position of the ULS icon.

https://github.com/wikimedia/jquery.ime is the code. And to clarify, it's the IME icon. ULS icon usually means the main entry point to ULS.

Quiddity renamed this task from CodeMirror shows ULS icon in wrong positions to CodeMirror shows IME icon in wrong positions.Oct 11 2017, 4:55 PM
kaldari triaged this task as High priority.Oct 17 2017, 10:46 PM
kaldari set the point value for this task to 5.
MaxSem claimed this task.Dec 12 2017, 7:53 PM
MaxSem moved this task from Ready to In Development on the Community-Tech-Sprint board.

Change 398602 had a related patch set uploaded (by MaxSem; owner: MaxSem):
[mediawiki/extensions/UniversalLanguageSelector@master] WIP: Add support for CodeMirror

https://gerrit.wikimedia.org/r/398602

Change 399460 had a related patch set uploaded (by MaxSem; owner: MaxSem):
[mediawiki/extensions/CodeMirror@master] Add UniversalLanguageSelector support

https://gerrit.wikimedia.org/r/399460

I applied both patches locally. The position for the selector appears correct but the transliteration doesn't work. I see English, no matter what language I pick. I tested in Chrome, Firefox and Safari. I've pulled both patches on Commtech wiki if anyone wants to test there. The result matches what I see locally.

http://commtech.wmflabs.org/w/index.php?title=Test

MaxSem removed MaxSem as the assignee of this task.Feb 22 2018, 7:46 PM
MaxSem added a subscriber: MaxSem.

I applied both patches locally. The position for the selector appears correct but the transliteration doesn't work. I see English, no matter what language I pick. I tested in Chrome, Firefox and Safari. I've pulled both patches on Commtech wiki if anyone wants to test there. The result matches what I see locally.
http://commtech.wmflabs.org/w/index.php?title=Test

Seems to work for me. It doesn't change the language of the interface, but I assume here it's meant only to change the language of the input, right? The only thing I see on CommTechWiki that differs from production is the corrected positioning. Tested in Chrome and Firefox.

I applied both patches locally. The position for the selector appears correct but the transliteration doesn't work. I see English, no matter what language I pick. I tested in Chrome, Firefox and Safari. I've pulled both patches on Commtech wiki if anyone wants to test there. The result matches what I see locally.
http://commtech.wmflabs.org/w/index.php?title=Test

Seems to work for me. It doesn't change the language of the interface, but I assume here it's meant only to change the language of the input, right? The only thing I see on CommTechWiki that differs from production is the corrected positioning. Tested in Chrome and Firefox.

Interesting. It doesn't change the input language for me. I'll ask @kaldari to give it a try.

kaldari added a subscriber: dchan.Mar 9 2018, 1:43 AM

I'm probably not going to have time to help with this. I wonder if maybe @dchan would be interested in helping to troubleshoot this. It's related to both language input and editing :)

We need someone from the Language team to help us test this.

@santhosh: Unfortunately, none of us really know how IME is supposed to work. Can you compare the current functioning of IME + Codemirror on the production wikis with a local IME + CodeMirror with https://gerrit.wikimedia.org/r/#/c/399460/ and https://gerrit.wikimedia.org/r/#/c/398602/ applied and let us know if it's an improvement?

dchan added a comment.EditedMar 14 2018, 4:38 AM

Hi, it's currently not working for me on either Firefox 57 or Chromium 63, both on Ubuntu. Here are exact steps to reproduce:

  1. Set up local wiki with https://gerrit.wikimedia.org/r/#/c/398602/4/ and https://gerrit.wikimedia.org/r/#/c/399460/4/ .
  2. Edit a page, type "== Test ==" and see the equals signs turn blue, proving that CodeMirror is loaded.
  3. In the lower right-hand corner of the edit window, see the small IME selection icon (keyboard + downarrow), proving that jQuery.IME is loaded.
  4. Click the icon, choose "..." under Other languages, and the language selector should appear.
  5. Type "no" and choose "norsk bokmål". The language selector should disappear.
  6. Click the icon again and choose "Normal transliterasjon" under "norsk bokmål". See that "Normal transliterasjon" now appears beside the icon.
  7. Type "Paa".

Expected behaviour: When you press 'a' the second time, the text changes into the Norwegian word "På".

Actual behaviour: The text "Paa" shows without change.

dchan added a comment.Mar 14 2018, 5:39 AM

Ok, I put some small fixes into https://gerrit.wikimedia.org/r/#/c/398602/5/ and https://gerrit.wikimedia.org/r/#/c/399460/5/ .

In Firefox, that seems to be enough: at least, "Paa" changes to "På".

In Chromium, there seems to be a more fundamental issue: the keypress event listener runs *after* the keystroke has already changed the content of the text area. That's not the way keypress events normally work (outside CodeMirror I mean), and so understandably the jQuery.IME code gets confused: "Pa" changes to "På", which is wrong, because the "a" is getting counted twice.

Does anyone know enough about CodeMirror to know if it has a neat way round this? :-) If not, we could code around it in jQuery.IME by seeing whether the text has changed since the keydown event, and then calculating accordingly.

Oh, the keypress behaviour we're seeing in Chromium doesn't seem to happen on the CodeMirror demo, either on https://codemirror.net/index.html (presumably the current version 5.35.0) or bundled in http://codemirror.net/codemirror-5.30.0.zip (which is the version the mediawiki extension currently includes).

Steps to reproduce:

  1. Load your CodeMirror instance in Chromium
  2. Open the Chromium console
  3. Type: document.getElementsByClassName( 'CodeMirror' )[ 0 ].addEventListener( 'keypress', function () { debugger; } )
  4. Click on the CodeMirror edit area and press 'x'

Expected behaviour: The debugger pauses before the 'x' appears in the edit area.

Actual behaviour on https://codemirror.net/index.html : Exactly as expected.
Actual behaviour on index.html inside http://codemirror.net/codemirror-5.30.0.zip : Exactly as expected.
Actual behaviour on CodeMirror within mediawiki: The debugger pauses after the 'x' appears in the edit area.

@dchan Thanks for working on this.

Oh, the keypress behaviour we're seeing in Chromium doesn't seem to happen on the CodeMirror demo, either on https://codemirror.net/index.html (presumably the current version 5.35.0) or bundled in http://codemirror.net/codemirror-5.30.0.zip (which is the version the mediawiki extension currently includes).
Steps to reproduce:

  1. Load your CodeMirror instance in Chromium
  2. Open the Chromium console
  3. Type: document.getElementsByClassName( 'CodeMirror' )[ 0 ].addEventListener( 'keypress', function () { debugger; } )
  4. Click on the CodeMirror edit area and press 'x'

Expected behaviour: The debugger pauses before the 'x' appears in the edit area.
Actual behaviour on https://codemirror.net/index.html : Exactly as expected.
Actual behaviour on index.html inside http://codemirror.net/codemirror-5.30.0.zip : Exactly as expected.
Actual behaviour on CodeMirror within mediawiki: The debugger pauses after the 'x' appears in the edit area.

I didn't have Chromium but I had an outdated version of Chrome and I tried the above steps in it (using CodeMirror on enwiki). The debugger paused before 'x' appears in the edit area, as it should have.
Then I updated my Chrome version to be sure and (tada!) now the debugger pauses after 'x' appears in the edit area.
In Safari too it pauses after the character appears.

Ok, I put some small fixes into https://gerrit.wikimedia.org/r/#/c/398602/5/ and https://gerrit.wikimedia.org/r/#/c/399460/5/ .
In Firefox, that seems to be enough: at least, "Paa" changes to "På".
In Chromium, there seems to be a more fundamental issue: the keypress event listener runs *after* the keystroke has already changed the content of the text area. That's not the way keypress events normally work (outside CodeMirror I mean), and so understandably the jQuery.IME code gets confused: "Pa" changes to "På", which is wrong, because the "a" is getting counted twice.
Does anyone know enough about CodeMirror to know if it has a neat way round this? :-) If not, we could code around it in jQuery.IME by seeing whether the text has changed since the keydown event, and then calculating accordingly.

Maybe @MusikAnimal will be able to provide some feedback and/or code review?

I just checked out those patches, and in Chrome indeed I see that a single a becomes å, when I take it that should only happen after typing a second a. I also see the debugger gets called after the character is inserted.

I'm not very familiar with the CodeMirror code beyond the syntax highlighting, but I will try to debug and figure out what's going on.

@dchan I looked into this more, and first off with https://gerrit.wikimedia.org/r/#/c/398602/5/ and https://gerrit.wikimedia.org/r/#/c/399460/5/ I've found Firefox on MacOS High Sierra has the same issue that you describe with Chromium. That is, a single a becomes å.

All the keystroke things I think happen in codemirror.js (the core library, not MediaWiki). I was able to put a debugger in there and see that it is called before the character is inserted. Perhaps the listener you're adding is getting fired after the CodeMirror's keypress event, which adds the character? So in other words, this could be a race condition.

Would listening to keydown work? I see that reliably gets called before CodeMirror inserts the character.

Niharika renamed this task from CodeMirror shows IME icon in wrong positions to CodeMirror should support IME functionality.Apr 10 2018, 9:24 PM

@kaldari — Niharika and I are discussing how to make progress with this task. It seems to be a hot potato that no CommTech wants to solve, but it is one of the final release blockers for CodeMirror.

dchan added a comment.Apr 11 2018, 1:16 AM

@MusikAnimal I think listening to keydown might work here, but might break other uses of jQuery.IME . If it does work here, perhaps we could consider making it an option. Anyone want to try changing the binding and see whether it works?

I'm going to take another stab at this tomorrow. I still think it's a race condition, where CodeMirror's keypress event is getting fired first. Maybe we could introduce a "hook", so to speak, that IME could use? This would mean modifying the core codemirror.js, but it seems something like this might be worth committing upstream anyway. Other editing libraries that work off of keypress might have the same issue.

dchan added a comment.Apr 11 2018, 4:41 AM

That sounds like a good idea, because even if we switched to listening for keydown, we'd be depending on our keydown listener to run first, which would be quite brittle.

Thank you @dchan and @MusikAnimal — looking forward to having this gnarly issue resolved! 🐛

MusikAnimal updated the task description. (Show Details)Apr 30 2018, 4:20 AM

@dchan @santhosh and @Arrbee — The Community Tech team is at a loss on this task. @MusikAnimal has been digging in deep and can't seem to find a way to make CodeMirror syntax highlighting work with the ULS. We either need to consider disabling support the ULS when CodeMirror is enabled or having another team with more experience with the ULS take a look.

Leon — can you please leave your comments of what you've found and what obstacles you encountered?

Sure, I will try to explain. The initial issue I think we were having is that the CodeMirror keypress event gets fired before the ULS event, as described at T171374#4048732.

First, I tried listening to the documented CodeMirror DOM events, but they didn't appear to work as desired. I might have been doing it wrong, not sure, but anyway going that route I still found that the CodeMirror event happened first which doesn't help us.

So, just to test things, I hacked in a custom jQuery event in CodeMirror's onKeyPress method, e.g. $( '.CodeMirror-code' ).trigger('uls-keypress', e);. Then we listen to that in IME.prototype.listen, something like:

$( '.CodeMirror-code' ).on( 'uls-keypress', function ( _uls, event ) {
	this.keypress( event );
}.bind( this ) );

Viola, ULS now has control and the transliteration correctly happens on the second keystroke (using the norsk bokmål example T171374#4048686). However there is still an additional a character, so typing aa gives you åa. As far as I can tell, CodeMirror isn't intentionally adding this. Every event (keypress, click, etc.) will ultimately call endOperation. If I put a console log in there, I see that after typing the second a, there are two hits, only one where cm.curOp.viewChanged and cm.curOp.typing are true.

The flow of execution in codemirror.js is very complex and hard to debug. I've sort of gone in circles trying to find a solution, to no avail. Hopefully my findings are at least helpful!

TBolliger renamed this task from CodeMirror should support IME functionality to Make CodeMirror support IME functionality.May 7 2018, 11:52 PM
TBolliger removed a project: Community-Tech-Sprint.

Due to the difficulty of these defects, the minimal usage of IE, and the fact this is an opt-in feature, the Community Tech team will not be fixing this defect. Rather, we will disable IME/ULS when CodeMirror is enabled. We'll file another task to this effect.

mxn added a subscriber: mxn.May 8 2018, 5:41 AM
mxn added a comment.May 8 2018, 6:34 AM

I'm going to take another stab at this tomorrow. I still think it's a race condition, where CodeMirror's keypress event is getting fired first. Maybe we could introduce a "hook", so to speak, that IME could use? This would mean modifying the core codemirror.js, but it seems something like this might be worth committing upstream anyway. Other editing libraries that work off of keypress might have the same issue.

By the way, the AVIM gadget listens for keypress and works fine in VisualEditor, but it experiences similar problems in CodeMirror when embedded in the old wikitext editor. (The Firefox extension also supports the Ace editor used by the CodeEditor extension.) Fortunately, if you enable the 2017 wikitext editor, AVIM ends up working just fine, because it uses the VisualEditor environment. Perhaps it’s the same situation with jquery.ime?

Change 399460 abandoned by MaxSem:
Add UniversalLanguageSelector support

https://gerrit.wikimedia.org/r/399460

Niharika lowered the priority of this task from High to Normal.Jun 5 2018, 6:53 PM
Niharika removed a project: Patch-For-Review.
psubhashish1 added a subscriber: psubhashish1.EditedDec 28 2018, 6:50 PM

Tested on Odia Wiktionary on Firefox 63.0.3. Syntax Highlighting and ULS don't work at the same time. I had to disable Syntax Highlighting to be able to use ULS.

Change 398602 abandoned by MaxSem:
Add support for CodeMirror

https://gerrit.wikimedia.org/r/398602