Last Comment Bug 816756 - [meta] CodeMirror as an alternative to Orion
: [meta] CodeMirror as an alternative to Orion
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Developer Tools: Source Editor (show other bugs)
: unspecified
: x86 Mac OS X
: P2 normal with 2 votes (vote)
: ---
Assigned To: Anton Kovalyov (:anton)
:
: Gabriel Luong [:gl][1 biz day review guarantee] (ΦωΦ)
Mentors:
Depends on: 912260 919703 919706 919707 919709 919978 929234 929766 929887 932439 935694 938111 1005473
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-29 15:31 PST by Anton Kovalyov (:anton)
Modified: 2014-05-03 05:52 PDT (History)
33 users (show)
mgoodwin: sec‑review+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Proof-of-concept: CodeMirror 2 as our source editor. (168.30 KB, patch)
2012-11-29 15:31 PST, Anton Kovalyov (:anton)
no flags Details | Diff | Splinter Review
Proof-of-concept: CodeMirror 3 as out source editor. (214.40 KB, patch)
2012-12-05 16:35 PST, Anton Kovalyov (:anton)
no flags Details | Diff | Splinter Review
CodeMirror 3 as our source editor. (255.42 KB, patch)
2012-12-27 18:26 PST, Anton Kovalyov (:anton)
no flags Details | Diff | Splinter Review
CodeMirror 3.1 as the default source editor (320.82 KB, patch)
2013-03-18 17:49 PDT, Anton Kovalyov (:anton)
no flags Details | Diff | Splinter Review
WIP 1 (272.02 KB, patch)
2013-08-14 18:49 PDT, Anton Kovalyov (:anton)
no flags Details | Diff | Splinter Review
WIP 2 (275.85 KB, patch)
2013-08-14 19:32 PDT, Anton Kovalyov (:anton)
no flags Details | Diff | Splinter Review
WIP 3 (292.91 KB, patch)
2013-08-15 17:07 PDT, Anton Kovalyov (:anton)
no flags Details | Diff | Splinter Review

Description Anton Kovalyov (:anton) 2012-11-29 15:31:21 PST
Created attachment 686837 [details] [diff] [review]
Proof-of-concept: CodeMirror 2 as our source editor.

This bug is an experiment to see how well CodeMirror 2 stacks up to Orion for our Source Editor needs. I understand that there were many hours spent on integrating Orion but I think it is worth it to do a comparison.

Attached patch is a proof-of-concept. Since SE was nicely written with multiple editors in mind all I had to do is to write another SourceEditor object that can be enabled via the devtools.editor.component config.

I ported just enough methods to make all Scratchpad tests to pass. Other tools (style editor, debugger, etc.) will most likely fail simply because I didn't get around porting all methods before creating this ticket. Will update the patch once I have more.

As you can see in source-editor-codemirror.jsm you don't really need to write lots of code to implement all the features we need. Most methods are two-three liners.

That said, we _will_ need to compare both editors on large minified (emscripten generated) scripts. When I was profiling debugger for bug 757408 the main culprit was our source editor.

Advantages:

 * Only 3k lines of code.
 * Author (Marijn Haverbeke, CC'ed here) is a mozillian and is willing to personally help with integration.
 * Has nice support for inline widgets and marker ranges[1] which allows us to write things like Irakli's interactivate[2] and other cool stuff.
 * WebKit devs did a similar experiment and reported that CodeMirror's runtime footprint and file loading speed was better than their old editor[3]. Of course, we will have to run our own benchmarks.
 * (subjective) Source is easier to understand.

Disadvantages:

 * Lots of hours spent on integrating Orion.
 * Mihai and others are familiar with Orion code base.
 * CodeMirror was already considered in past but the team found that it lacked some features[4] (I couldn't find what features specifically).
 * + Add more in comments. My only experience with Orion so far was in bug 737803 and bug 757408. See this comment[5] for some of Orion's limitations. 

[1] http://marijnhaverbeke.nl/blog/codemirror-marked-ranges.html
[2] http://jeditoolkit.com/2012/11/12/interactivate.html
[3] https://bugs.webkit.org/show_bug.cgi?id=92769
[4] https://wiki.mozilla.org/DevTools/Features/CodeEditor
[5] https://bugzilla.mozilla.org/show_bug.cgi?id=737803#c7
Comment 1 Victor Porof [:vporof][:vp] 2012-11-30 00:00:50 PST
Apart from Anton's comments, here are my observations/2c:

* Scrolling is much smoother in CM
* It seems to have a more comprehensive syntax highlighter (knows about variables, params, differentiates between function calls, property access, numerals etc.), with, apparently, a faster engine [1]
* Some Orion bugs aren't a problem (for example, variable height lines, or adding breakpoints by clicking on line numbers [2])
* Code folding works not only for comments [3]

However, some of these issues may be fixed by future Orion updates.

[1] http://codemirror.net/demo/loadmode.html
[2] http://codemirror.net/demo/marker.html
[3] http://codemirror.net/demo/folding.html
Comment 2 Victor Porof [:vporof][:vp] 2012-11-30 00:11:47 PST
(In reply to Anton Kovalyov (:anton) from comment #0)
> I couldn't find what features specifically

IIRC, https://wiki.mozilla.org/DevTools/Features/CodeEditor#Release_Requirements
Comment 3 Marijn Haverbeke 2012-11-30 01:01:03 PST
(I'm the author of CodeMirror. I'd be delighted if this would land.)

One thing that will probably have to be resolved before this is production ready is the handling of very long lines. A few thousand characters should work fine with current CodeMirror, but if you open a big, minified library, getting ~50k characters on a single line is not a far-fetched scenario. That will currently make CodeMirror rather unresponsive (though the situation on FF is much better than on IE/Opera). I've filed a bug on this problem: https://github.com/marijnh/CodeMirror/issues/1022
Comment 4 Rob Campbell [:rc] (:robcee) 2012-11-30 15:11:43 PST
(In reply to Marijn Haverbeke from comment #3)
> (I'm the author of CodeMirror. I'd be delighted if this would land.)
> 
> One thing that will probably have to be resolved before this is production
> ready is the handling of very long lines. A few thousand characters should
> work fine with current CodeMirror, but if you open a big, minified library,
> getting ~50k characters on a single line is not a far-fetched scenario. That
> will currently make CodeMirror rather unresponsive (though the situation on
> FF is much better than on IE/Opera). I've filed a bug on this problem:
> https://github.com/marijnh/CodeMirror/issues/1022

There are issues with long lines in Orion as well. Some informal testing between CM and Orion this afternoon in Scratchpad with some minified JS showed them to be about equivalent (CM might have been a little better than the current version of Orion?) at inserting into a long line.

Nice to see this. We'll be able to do some decent comparisons with these available.
Comment 5 Jan Honza Odvarko [:Honza] 2012-12-05 05:59:33 PST
Using CM is also interesting for Firebug since Orion requires high privileges (which blocks Firebug from using it bug 787086).

I tried to apply the patch (http://hg.mozilla.org/integration/fx-team and http://hg.mozilla.org/mozilla-central/) but it fails when patching EventEmitter.jsm

Any chance to get an updated patch?

Honza

$ patch -p1 --dry-run < sourceeditor-codemirror.patch
patching file `browser/devtools/jar.mn'
Hunk #1 succeeded at 20 with fuzz 2 (offset 1 line).
patching file `browser/devtools/scratchpad/scratchpad.js'
Hunk #1 succeeded at 133 (offset 2 lines).
patching file `browser/devtools/scratchpad/test/browser_scratchpad_bug_669612_un
saved.js'
patching file `browser/devtools/shared/EventEmitter.jsm'
Hunk #1 FAILED at 1.
Hunk #2 FAILED at 52.
Hunk #3 FAILED at 70.
3 out of 3 hunks FAILED -- saving rejects to browser/devtools/shared/EventEmitte
r.jsm.rej
patching file `browser/devtools/sourceeditor/Makefile.in'
patching file `browser/devtools/sourceeditor/codemirror/codemirror.css'
patching file `browser/devtools/sourceeditor/codemirror/codemirror.js'
patching file `browser/devtools/sourceeditor/codemirror/javascript.js'
patching file `browser/devtools/sourceeditor/source-editor-codemirror.jsm'
patching file `browser/devtools/sourceeditor/source-editor-orion.jsm'
patching file `browser/devtools/sourceeditor/source-editor-ui.jsm'
patching file `browser/devtools/sourceeditor/source-editor.jsm'
Comment 6 Anton Kovalyov (:anton) 2012-12-05 10:36:56 PST
This patch has pieces of devtools-window that I need to take out since devtools-window is merged in now. I'll update it today.
Comment 7 Anton Kovalyov (:anton) 2012-12-05 16:35:25 PST
Created attachment 689012 [details] [diff] [review]
Proof-of-concept: CodeMirror 3 as out source editor.

Rebased with the latest changes from fx-team (incl. devtools-window) and swapped CodeMirror 2 with CodeMirror 3 (release candidate 2). Among other things it has bi-directional text support.

CodeMirror 3 has its own isClean/markClean methods so I probably don't need to implement our own dirty/_updateDirty. I'll investigate if we can use those later.
Comment 8 Kevin Dangoor 2012-12-06 09:03:35 PST
When evaluating editors initially, the specific areas that led us to Orion were:

* rtl text
* accessibility

Ehsan saw Orion's use of contentEditable as positive because it allowed Orion to lean on the browser a bit for those things. However, as I understand it, Orion still needed to override a fair bit of behavior in order to be a decent code editor.

IBM has a commitment to make Orion as accessible as possible.

That said, I don't know how far Orion has come wrt accessibility and I don't know how CM fares in that regard either.
Comment 9 Anton Kovalyov (:anton) 2012-12-06 09:06:03 PST
Ok, since CM3 has RTL support now we are down to accessibility and performance comparison.
Comment 10 Mihai Sucan [:msucan] 2012-12-06 09:34:12 PST
Didn't yet have time to test this patch - will do soon (I hope). Just some thoughts: as Kevin pointed out rtl support and accessibility were both the main reasons why Orion was chosen. However both capabilities do not work very well in Orion. RTL support is buggy and accessibility was problematic - I know they recently did improvements and we'll need to retest.

I'm tempted by CM2/3 because its codebase is probably more suited for what we need: easy pick-and-choose the features you want, easy to customize - I'll look into this aspect. Picking features we want from Orion's codebase is not so easy - without patching, or without making ugly workarounds.

On performance: very long lines always rendered very slow in Orion and CM2. There are known Gecko bugs related to this issue: bug 72885, bug 28783 and more. If you try loading one of the biggest stylesheets from github - just open it in a new tab - you will see how slow Firefox renders the file - and that's just plain text, no syntax highlighting.

The only way to work around this performance issue is to do (1) line wrapping - which current Orion versions implement (I am curious to see how well it works); or (2) partially render lines. Similar to how modern editors only put in DOM the visible lines, to avoid performance issues, they could also limit the number of characters in a line, based on the viewport size and scroll.
Comment 11 Mihai Sucan [:msucan] 2012-12-10 13:48:29 PST
Thanks for this patch Anton.

I looked into CodeMirror 3's code and I have the following comments:

* I like the source code overall. I would appreciate if it would have more
  documentation for private functions. Some function names are not self descriptive.
* No use of modular script loaders, like RequireJS and friends. Not bad, but that's
  becoming a good way to load modules and structure things around.
* Use of globals without easy way to override them. (fixed in Orion)
* Use of createElement() without a namespace - causes problems with documents that do
  not use XHTML as their default namespace. (fixed in Orion)

What follows is a comparison to the latest Orion codebase. What we have in Firefox is old (at this point).

Informal performance testing:

* without line wrapping:
  * scrolling seems to be similar. cm3 feels slightly faster.
  * typing is similar, in some cases one is slightly faster than the other.
  * keyboard-based cursor navigation seems faster in orion, in most cases.
  * mouse-based cursor navigation and selection are faster in cm3.
* with line wrapping enabled: cm3 is quite faster and usable.
* cm3 stops highlighting very long lines.

Native behavior:

* window.getSelection() is not updated. Native selection is not used.
  * this most-likely prevents native behavior from kicking-in, like select and
    copy to X11 buffer.
  * probably has negative impact for accessibility as well.
* one important concern we had with Orion was matching native editor behavior, on
  Macs, Linux and Windows. It looks like cm3 seems to have a number of Mac-specific
  keyboard shortcuts - which is good.

RTL support:

* it seems buggier in cm3 than in orion.
* arrow-based navigation direction is not inverted in RTL text.
  * gecko does not invert direction, only when shift is held down.
  * gedit does like gecko.
  * libreoffice writer:
    * if you specifically ask for rtl lines, then direction is not inverted when
    you write rtl texts. if you write ltr texts, arrow direction inversion
    happens.
    * if you have ltr lines and you write rtl, then arrow direction invertion
      happens.
  * orion does arrow direction inversion.
  * not sure which approach is best here. thoughts?
* RTL-only lines are not right-aligned automatically cm3.
  * gecko doesn't do this either.
  * gedit does this.
  * libreoffice write doesn't do it either.
  * orion doesn't do it.
  * i feel cm3 should do it because it's a plaintext editor, like gedit. any thoughts?
* caveats: i'm learning arabic and i'm also learning which is the best software
  with rtl support.
  * native rtl users should be asked about the best behavior.
  * did you maintain contact with rtl users to see how well cm3 does with rtl texts?

(these questions are for Marijn)

I looked into the CodeMirror 3 integration patch and I have the following
comments:

* noticed no context menu in scratchpad.
* a lot of trailing white spaces in the patch.
* missing source editor functionality, but it looks very good overall.
* source-editor-codemirror.jsm needs code comments.
* most of these are simply because the patch is still Work In Progress, so no
  worries Anton.

CodeMirror 3 issues I'd like to see fixed: #1079, #1045, #931, #598, #249. (at a quick glance)

Overall: codemirror3 is tempting. I like the improved performance with line wrapping enabled. That big difference makes me wonder if there isn't a mistake in the Orion line wrapping code - something they could, perhaps, easily fix when we report the bug.

Not much to add, for now. Thank you!
Comment 12 Anton Kovalyov (:anton) 2012-12-10 18:35:17 PST
Thanks, Mihai! Yeah, trailing whitespace and other minor issues are because this patch is WIP. I'll take a look why there is no context menu.
Comment 13 Marijn Haverbeke 2012-12-11 01:58:50 PST
> * No use of modular script loaders, like RequireJS and friends. Not bad, but that's
>   becoming a good way to load modules and structure things around.

I currently think that making the library trivial to include is still outweighing the benefits of splitting up the core library into files, which would require more work for end users. As the code grows further, I might revisit this decision.

> * Use of globals without easy way to override them. (fixed in Orion)

Can you elaborate on that? Do you mean the 'CodeMirror' global? What would be the reason for overriding it?

> * Use of createElement() without a namespace - causes problems with documents that do
>   not use XHTML as their default namespace. (fixed in Orion)

That'd be an easy thing to patch (which I'd gladly merge in when submitted).

>   * keyboard-based cursor navigation seems faster in orion, in most cases.

I'd be interested in a description of a situation where keyboard navigation is noticeably slow.

> * cm3 stops highlighting very long lines.

This is a hack to reduce the amount of work needed to do on such lines. It'd be easy to disable or make configurable, if desired.

> * window.getSelection() is not updated. Native selection is not used.
>   * this most-likely prevents native behavior from kicking-in, like select and
>     copy to X11 buffer.

Indeed. On Webkit, programmatically selecting something in a textarea will update the X11 buffer, and middle-click fires a paste event that I can use to switch focus to the textarea and make middle-click paste work. In Firefox, neither of these are present, so the X11 buffer isn't working will with CodeMirror.

>  * probably has negative impact for accessibility as well.

I'd really like to hear details about what kind of accessibility we're talking about. Since Firefox is not a screen reader, that scenario seems to be out of scope.

> * it seems buggier in cm3 than in orion.

Please report bugs! (Thanks for the one you already reported.) I'm just an ignorant Western European, and it seems I haven't been able to get anyone to solidly test RTL support since I wrote it, so bugs will be found. And they will be fixed.

> * arrow-based navigation direction is not inverted in RTL text.

I spent a lot of time to make it work that way (matching Webkit and most non-Windows software), since it seems saner to me -- there's an actual directional arrow on the arrow keys, which suggests a visual direction of movement. The 'naive' approach, where arrows just move by string index rather than screen position, could be made an option if desired. It's easy to do (compare the moveLogically and moveVisually internal functions).

>  * did you maintain contact with rtl users to see how well cm3 does with rtl texts?

I made some attempts but didn't find anyone who really did serious testing. If you know candidates, point them in the direction of CM. As mentioned above, bug reports on this are extremely welcome.
Comment 14 simon.kaegi 2012-12-11 08:09:45 PST
Agree -- thanks Mihai -- good coverage of RTL
Another related area to consider is multi-key characters, variable width characters, and cursor support.

re: peformance of line-wrapping
Our line wrapping and variable line height code is relatively new and for our 1.0 release we focused more on style correctness than performance. I definitely agree that there is some low hanging fruit in terms of optimization and these will land shortly.

re: very long lines
Support for very long lines is another bug being looked at using the same view port approach we use for the rest of the editor's contents. This is a significant change and will probably take a number of weeks work. If this is causing real problems (and apparently it is) we can look at doing an interim fix to stop styling after a character threshold and then chunk render the unstyled text so there is minimal performance degradation.

--

The Orion team remains committed to producing software that is accessible and that goes for the whole of Orion and not just the editor component. We dedicate fulltime resources to ensure all of Orion is keyboard accessible and useable by screen readers like JAWS and NVDA. I think providing accessibility is so important and would encourage anyone looking at this bug to spend a little time learning (http://wiki.eclipse.org/Orion/Testing_Orion_Accessibility)

We also remain committed to using "contenteditable" as the basis of our editor. Kevin is absolutely accurate when he says we end up over-riding behavior to give a reasonable user experience however where-ever we can we are leveraging the browser native capabilities. Today that more or less boils down to cursor support and some aspects of input and layout however as the situation in the browser gets better so will we. Improving the browser's native edit APIs is an active area and one we hope we can work with Mozilla and the other browsers to help standardize through the W3C/WHATWG. If in ten years time we are still having the discussion about which low-level text widget to support I think we will have done a disservice to app developers.
Comment 15 Marijn Haverbeke 2012-12-11 08:20:10 PST
> Another related area to consider is multi-key characters, variable width characters, and cursor support.

CM handles multi-key characters (IME and compose-key style) and variable-width characters. What do you mean by cursor support?
Comment 16 simon.kaegi 2012-12-11 08:36:18 PST
(In reply to Marijn Haverbeke from comment #15)
> CM handles multi-key characters (IME and compose-key style) and
> variable-width characters.
Nice work (especially on variable width fonts). I'll take a look.

> What do you mean by cursor support?
I believe CM draws its own cursor. I'm just referring to correct cursor behavior which is one of the things we get for free.
Comment 17 Thaddee Tyl [:espadrine] 2012-12-11 10:03:32 PST
(In reply to simon.kaegi from comment #16)
> > What do you mean by cursor support?
> I believe CM draws its own cursor. I'm just referring to correct cursor
> behavior which is one of the things we get for free.

I can't really see how the cursor could behave in a wrong way.
On the other hand, CodeMirror's design allows for multiple cursors in the future [1],
à la Sublime Text (a feature which has also made its apparition in the Ace text editor).

  [1]: https://groups.google.com/forum/?fromgroups=#!topic/CodeMirror/nGGBGuNrFas

On the subject of accessibility, I admit I know little, but I am quite interested.
On which grounds should an editor be judged? Just the ability to let programs read the textual content?
(I've tried to run Orca to test CodeMirror; that program confuses me to no end. I can't even make it read its own documentation.)

Which ARIA states should appear?
There aren't any widgets in raw CodeMirror.
Comment 18 Mihai Sucan [:msucan] 2012-12-11 10:06:02 PST
(In reply to Marijn Haverbeke from comment #13)
> > * No use of modular script loaders, like RequireJS and friends. Not bad, but that's
> >   becoming a good way to load modules and structure things around.
> 
> I currently think that making the library trivial to include is still
> outweighing the benefits of splitting up the core library into files, which
> would require more work for end users. As the code grows further, I might
> revisit this decision.

Not too important right now.

> > * Use of globals without easy way to override them. (fixed in Orion)
> 
> Can you elaborate on that? Do you mean the 'CodeMirror' global? What would
> be the reason for overriding it?

I mean the rest of the globals: window, document, etc. We should be able to load CodeMirror into an environment without any DOM objects, without any browser-like objects. On initialization we should be able to give it the window object from where to take whatever it needs.

This would allow us to package codemirror as jsm (JavaScript module) that we load once, outside of a browser iframe. Whenever we need to create an editor instance we would give it a target, a window object, etc - all that it needs.

> > * Use of createElement() without a namespace - causes problems with documents that do
> >   not use XHTML as their default namespace. (fixed in Orion)
> 
> That'd be an easy thing to patch (which I'd gladly merge in when submitted).

Thanks!

> >   * keyboard-based cursor navigation seems faster in orion, in most cases.
> 
> I'd be interested in a description of a situation where keyboard navigation
> is noticeably slow.

I tested with a script made from a YUI script:

http://mihaisucan.github.com/mozilla-work/stress.js

> > * cm3 stops highlighting very long lines.
> 
> This is a hack to reduce the amount of work needed to do on such lines. It'd
> be easy to disable or make configurable, if desired.

This seems sensible.

> > * window.getSelection() is not updated. Native selection is not used.
> >   * this most-likely prevents native behavior from kicking-in, like select and
> >     copy to X11 buffer.
> 
> Indeed. On Webkit, programmatically selecting something in a textarea will
> update the X11 buffer, and middle-click fires a paste event that I can use
> to switch focus to the textarea and make middle-click paste work. In
> Firefox, neither of these are present, so the X11 buffer isn't working will
> with CodeMirror.

Please file bugs. For Orion I did it using their selection event from chrome-privileged code - this is what would be needed for cm3 as well.

> >  * probably has negative impact for accessibility as well.
> 
> I'd really like to hear details about what kind of accessibility we're
> talking about. Since Firefox is not a screen reader, that scenario seems to
> be out of scope.

There are screen readers outside of Mozilla which read text from most software, including Firefox. The lack of an actual selection will definitely confuse screen reading software which will not be able to determine which text is selected. The actual cursor location is also a problem - screen readers use the Firefox native cursor location to tell the user their location in an editable.

> > * it seems buggier in cm3 than in orion.
> 
> Please report bugs! (Thanks for the one you already reported.) I'm just an
> ignorant Western European, and it seems I haven't been able to get anyone to
> solidly test RTL support since I wrote it, so bugs will be found. And they
> will be fixed.

Thank you for the quick fix.

> > * arrow-based navigation direction is not inverted in RTL text.
> 
> I spent a lot of time to make it work that way (matching Webkit and most
> non-Windows software), since it seems saner to me -- there's an actual
> directional arrow on the arrow keys, which suggests a visual direction of
> movement. The 'naive' approach, where arrows just move by string index
> rather than screen position, could be made an option if desired. It's easy
> to do (compare the moveLogically and moveVisually internal functions).

I would personally prefer the cm3 approach: always move in the visual direction. Direction inversion is confusing, BUT don't take my preference as relevant because I am not a native RTL users.

> >  * did you maintain contact with rtl users to see how well cm3 does with rtl texts?
> 
> I made some attempts but didn't find anyone who really did serious testing.
> If you know candidates, point them in the direction of CM. As mentioned
> above, bug reports on this are extremely welcome.

I'm tempted to add someone here to CC, but I'd like us to avoid making this bug a place to discuss every possible problem. I suggest we move the RTL discussion into a github issue for the codemirror project and I will try to bring someone more knowledgeable.

Thank you!
Comment 19 Dave Camp (:dcamp) 2012-12-11 11:36:14 PST
Any thoughts on which will be easier to keep up-to-date in our source tree?
Comment 20 simon.kaegi 2012-12-11 12:17:28 PST
(In reply to Thaddee Tyl [:espadrine] from comment #17)
> On the subject of accessibility, I admit I know little, but I am quite
> interested.
> On which grounds should an editor be judged? Just the ability to let
> programs read the textual content?

Thanks Thaddee and I'm glad you're interested. I too am by no means an expert and one of the problems is that there is no authorative document on the subject. With that said I believe that having an accessible editor widget is important and something worth tackling.

Here's a few more links that I've looked at:
http://www.w3.org/TR/ATAG10-TECHS/imp7
http://help.eclipse.org/juno/index.jsp?topic=%2Forg.eclipse.platform.doc.user%2Fconcepts%2Faccessibility%2Ftext_editor.htm

It would be great if someone more knowledgeable could pipe in but to me the most critical features are:
1) all functionality is keyboard accessible
2) support for high contrast and magnification
3) Screen readers should not get lost when navigating content (e.g. reading the wrong lines/characters and messing up input)
4) Support for navigation cues on movement and in some cases line number cues
5) Correct support for character selection, word selection, cut, copy, paste, and delete

If those five were handled reasonably well by web editors we'd be in much better shape than we are currently. The Orion editor is by no means perfect at handling this stuff but we test, fix bugs and are committed to providing something that meets these requirements.
Comment 21 Mihai Sucan [:msucan] 2012-12-11 12:32:42 PST
(In reply to Dave Camp (:dcamp) from comment #19)
> Any thoughts on which will be easier to keep up-to-date in our source tree?

This is a good question. I would find both similarly easy to maintain/update, until we take into consideration key aspects:

- project maturity: CodeMirror seems to be more evolved as a project (currently), less changing its public API. (just what I personally perceive, others might say otherwise.) Orion is currently evolving fast.

Down the road I expect Orion will have a clear upgrade path, release notes with API changes, and "corporate level" information on how to not break things.

- tests: both projects have automated tests that we would need to integrate. I'd like both projects to have a policy on writing tests for every bug fix their teams do - similar to what we do at Mozilla.

- native behavior: Orion is intricate in how it combines native behavior for selection, cursor, keyboard, etc, and it also overwrites many things. This is prone to errors, regressions whenever we change something in Gecko, etc.

This is the bit that worries me most. Hard to give a clear answer here.
Comment 22 Thaddee Tyl [:espadrine] 2012-12-11 14:00:45 PST
(In reply to Marijn Haverbeke from comment #13)
> > * arrow-based navigation direction is not inverted in RTL text.
> 
> I spent a lot of time to make it work that way (matching Webkit and most
> non-Windows software), since it seems saner to me -- there's an actual
> directional arrow on the arrow keys, which suggests a visual direction of
> movement. The 'naive' approach, where arrows just move by string index
> rather than screen position, could be made an option if desired. It's easy
> to do (compare the moveLogically and moveVisually internal functions).

I just talked to an arabic-speaking relative.
Counter-intuitively, the leftwards arrow goes one glyph to the right and vice-versa.
The backspace removes one character to the right of the cursor
(which is usually the one just typed).
That's another key whose arrow goes the "wrong" way in RTL.
Comment 23 Marijn Haverbeke 2012-12-12 01:52:37 PST
(In reply to Mihai Sucan [:msucan] from comment #18)
> I mean the rest of the globals: window, document, etc. We should be able to
> load CodeMirror into an environment without any DOM objects, without any
> browser-like objects. On initialization we should be able to give it the
> window object from where to take whatever it needs.

Ah, right. That'd be somewhat awkward, but definitely doable -- it'd add an extra argument to a lot of the utility functions. If we get to the point where this becomes a significant consideration, I'll gladly look into it. 

> I tested with a script made from a YUI script:

Ah, right, that's the overlong line issue I mentioned earlier.

> I would personally prefer the cm3 approach: always move in the visual
> direction. Direction inversion is confusing, BUT don't take my preference as
> relevant because I am not a native RTL users.

I've added an option. I followed the platform defaults as I perceive them -- on Windows, almost all apps invert movement in rtl text, on Linux and OS X, most apps follow a visual order (not inverting movement).

See http://codemirror.net/doc/manual.html#option_rtlMoveVisually
Comment 24 Silenio Quarti 2012-12-12 05:18:33 PST
(In reply to Mihai Sucan [:msucan] from comment #21)
> - native behavior: Orion is intricate in how it combines native behavior for
> selection, cursor, keyboard, etc, and it also overwrites many things. This
> is prone to errors, regressions whenever we change something in Gecko, etc.
> 
> This is the bit that worries me most. Hard to give a clear answer here.

Thanks Mihai for describing the strengths and weaknesses of the Orion editor so well.

As Simon said we really believe that leveraging the native user agent support is the best approach in the long run. As the user agent support evolve for things like Bidi, IME, accessibility, tablet support we tend to get it all for free.   I realize we override quite a bit of the native behavior on the desktop for better integration, but this is changing with time.  For example, in order to support iPad and Android, we are relying on the user agent for selection and cursor navigation more.  This allows us to get the caret and selection magnifiers on the iPad and to work with the swift keyboard on Android. As we iron out the outstanding issues, I can see the desktop code also taking advantage of this approach.
Comment 25 Mihai Sucan [:msucan] 2012-12-13 08:52:31 PST
(In reply to Marijn Haverbeke from comment #23)
> I've added an option. I followed the platform defaults as I perceive them --
> on Windows, almost all apps invert movement in rtl text, on Linux and OS X,
> most apps follow a visual order (not inverting movement).
> 
> See http://codemirror.net/doc/manual.html#option_rtlMoveVisually

Thank you!
Comment 26 Anton Kovalyov (:anton) 2012-12-27 18:26:26 PST
Created attachment 696190 [details] [diff] [review]
CodeMirror 3 as our source editor.

Style Editor tests now pass, I'm switching to Debugger tests.

Also, in addition to aforementioned Interactivate by Irakli I found a couple of more CodeMirror demos that can inspire new Scratchpad/Debugger/Style Editor/etc. features:

 * Inline hints: http://codemirror.net/demo/widget.html (we will probably use our built-in linter or something new on top of Reflect for this)
 * Code auto-formatting for minified HTML and JavaScript: http://codemirror.net/demo/formatting.html
Comment 27 :Ehsan Akhgari 2013-03-04 08:21:08 PST
I just tried entering some RTL text in the CodeMirror formatting demo and there is a double cursor bug when entering the text (which doesn't happen when moving the cursor around in RTL text.  I also tried entering some IME characters and while things seemed to work, I did not get the IME underlines, but I don't know how bad this is.  As far as I can tell, CodeMirror doesn't use the browser text input handling facilities, which is bad.  I also managed to confuse the text highlighting pretty easily by copying some JS string and pasting it inside the CSS subsection of the document in the demo.
Comment 28 Marijn Haverbeke 2013-03-04 08:25:53 PST
- If one of the cursors you saw was grey, that's not a bug -- when the cursor is on an RTL/LTR boundary, it shows both insertion points -- the one where an RTL character would end up, and the one where an LTR character would end up. If that is not what you saw, please file a bug, with instructions to reproduce, at https://github.com/marijnh/codemirror/issues

- CodeMirror does use a focused textarea to get input. I'm not sure how this is bad.

- If you paste garbage into an editor, yes, it will be highlighted as garbage. I'd say this is a feature. If you actually had correct code highlighted incorrectly, again, please file an issue.
Comment 29 :Ehsan Akhgari 2013-03-04 11:32:39 PST
(In reply to comment #28)
> - If one of the cursors you saw was grey, that's not a bug -- when the cursor
> is on an RTL/LTR boundary, it shows both insertion points -- the one where an
> RTL character would end up, and the one where an LTR character would end up. If
> that is not what you saw, please file a bug, with instructions to reproduce, at
> https://github.com/marijnh/codemirror/issues

Yes, that is what I saw, and it is most certainly a bug.  There is only ever one insertion point.  Neither Gecko nor any other application that I know of shows the other fictional insertion point.

> - CodeMirror does use a focused textarea to get input. I'm not sure how this is
> bad.

For one thing it probably doesn't go through part of IME insertion code path.  You need to check with Masayuki on the implications of that.

> - If you paste garbage into an editor, yes, it will be highlighted as garbage.
> I'd say this is a feature. If you actually had correct code highlighted
> incorrectly, again, please file an issue.

The entire block that I pasted was colored in a different color.  I don't remember exactly but I think this is what happened:

Before:

 .cl|ass { blah...}

After pasting

 .clfoo barass { blah...}
    ^^^^^^^
    pasted text

This is definitely a valid CSS selector.  The entire "foo bar" text had a different color (blue?)
Comment 30 Thaddee Tyl [:espadrine] 2013-03-04 11:43:11 PST
> --- Comment #29 from :Ehsan Akhgari <ehsan@mozilla.com> 2013-03-04 11:32:39 PST ---
> (In reply to comment #28)
>> - If you paste garbage into an editor, yes, it will be highlighted as garbage.
>> I'd say this is a feature. If you actually had correct code highlighted
>> incorrectly, again, please file an issue.
>
> The entire block that I pasted was colored in a different color.  I don't
> remember exactly but I think this is what happened:
>
> Before:
>
>  .cl|ass { blah...}
>
> After pasting
>
>  .clfoo barass { blah...}
>     ^^^^^^^
>     pasted text
>
> This is definitely a valid CSS selector.  The entire "foo bar" text had a
> different color (blue?)

Can you reproduce that?
I tried quite hard to do it, but it invariably works for me.
Which version of codemirror are you using? on which version of Firefox?
on which pages? on which OS?
Comment 31 :Ehsan Akhgari 2013-03-04 12:08:42 PST
(In reply to comment #30)
> > --- Comment #29 from :Ehsan Akhgari <ehsan@mozilla.com> 2013-03-04 11:32:39 PST ---
> > (In reply to comment #28)
> >> - If you paste garbage into an editor, yes, it will be highlighted as garbage.
> >> I'd say this is a feature. If you actually had correct code highlighted
> >> incorrectly, again, please file an issue.
> >
> > The entire block that I pasted was colored in a different color.  I don't
> > remember exactly but I think this is what happened:
> >
> > Before:
> >
> >  .cl|ass { blah...}
> >
> > After pasting
> >
> >  .clfoo barass { blah...}
> >     ^^^^^^^
> >     pasted text
> >
> > This is definitely a valid CSS selector.  The entire "foo bar" text had a
> > different color (blue?)
> 
> Can you reproduce that?
> I tried quite hard to do it, but it invariably works for me.
> Which version of codemirror are you using? on which version of Firefox?
> on which pages? on which OS?

It's not easy to reproduce but I didn't try too hard.  I'm running Nightly on Mac.  No idea about what version of CM, whatever the demo page uses.
Comment 32 Marijn Haverbeke 2013-03-05 01:03:58 PST
(In reply to :Ehsan Akhgari from comment #29)
> Yes, that is what I saw, and it is most certainly a bug.  There is only ever
> one insertion point.

If you have the string "fooكلمةbar", and the cursor is between the 'ة' and the 'b', typing a Latin character will cause the character to show up to the left of the 'b', whereas typing an Arabic character will cause it to show up to the right of the second 'o'. Thus, visually, there are two insertion points. This is what the secondary cursor shows.

>  Neither Gecko nor any other application that I know of
> shows the other fictional insertion point.

Chrome does it this way on Linux and OS X (though not on Windows).

> For one thing it probably doesn't go through part of IME insertion code
> path.  You need to check with Masayuki on the implications of that.

Did you even try this? IME, at least the form that a Chinese user explained to me, works with CodeMirror. I'm not an expert on this, so it might be that I'm missing some other use case. In that case, please point out the precise problem instead of confusing the communication with vague claims that something 'probably' does not work.

> The entire block that I pasted was colored in a different color.  I don't
> remember exactly but I think this is what happened:
> 
> Before:
> 
>  .cl|ass { blah...}
> 
> After pasting
> 
>  .clfoo barass { blah...}
>     ^^^^^^^
>     pasted text
> 
> This is definitely a valid CSS selector.  The entire "foo bar" text had a
> different color (blue?)

Okay. When I try this, and several variations of it, the highlighting is just fine.
Comment 33 :Ehsan Akhgari 2013-03-08 10:36:37 PST
(In reply to comment #32)
> (In reply to :Ehsan Akhgari from comment #29)
> > Yes, that is what I saw, and it is most certainly a bug.  There is only ever
> > one insertion point.
> 
> If you have the string "fooكلمةbar", and the cursor is between the 'ة' and the
> 'b', typing a Latin character will cause the character to show up to the left
> of the 'b', whereas typing an Arabic character will cause it to show up to the
> right of the second 'o'. Thus, visually, there are two insertion points. This
> is what the secondary cursor shows.

Yes, I'm well aware of that.  My point was that this is not how the bidi caret works in Gecko, so as far as integrating CM into the product, this won't be acceptable.

> >  Neither Gecko nor any other application that I know of
> > shows the other fictional insertion point.
> 
> Chrome does it this way on Linux and OS X (though not on Windows).

I just tested on OS X to ensure my own sanity, and as far as I can tell, it doesn't do that.  And even if it did, we probably wouldn't want to participate in inventing this concept.  ;-)

> > For one thing it probably doesn't go through part of IME insertion code
> > path.  You need to check with Masayuki on the implications of that.
> 
> Did you even try this? IME, at least the form that a Chinese user explained to
> me, works with CodeMirror. I'm not an expert on this, so it might be that I'm
> missing some other use case. In that case, please point out the precise problem
> instead of confusing the communication with vague claims that something
> 'probably' does not work.

I'm probably going to stop commenting here, as it seems like you're taking my comments personally.  I did try this, and if you read comment 27 carefully you'll see that I did point out the precise problem and also mentioned that I'm not sure if that's bad or not.  I'm not making any vague claims here, Marijn.  I'm commenting on the things that we would like to see fixed for CM integrated into the devtools.  Basically, we don't want different editing behavior in devtools code windows versus everywhere else in Gecko, so we need to modify CM where needed to change such types of behavior if needed.  As far as CM outside of Firefox Developer Tools is concerned, I have no strong opinions on how things should work there.

> > The entire block that I pasted was colored in a different color.  I don't
> > remember exactly but I think this is what happened:
> > 
> > Before:
> > 
> >  .cl|ass { blah...}
> > 
> > After pasting
> > 
> >  .clfoo barass { blah...}
> >     ^^^^^^^
> >     pasted text
> > 
> > This is definitely a valid CSS selector.  The entire "foo bar" text had a
> > different color (blue?)
> 
> Okay. When I try this, and several variations of it, the highlighting is just
> fine.

Yeah, unfortunately I can't reproduce this successfully either.
Comment 34 Marijn Haverbeke 2013-03-09 03:51:29 PST
> Yes, I'm well aware of that.  My point was that this is not how the bidi
> caret works in Gecko, so as far as integrating CM into the product, this
> won't be acceptable.

Well, before you were talking about a "fictional insertion point", insisting this definitely was a bug, generally giving off the impression that you weren't taking my explanation the least bit seriously. That's what made me react in a somewhat annoyed way.

It'd be a two-line patch to allow this behavior to be turned off with a configuration option, and I'll gladly make that change.

> I did try this [IME], and if you read comment 27
> carefully you'll see that I did point out the precise problem and also
> mentioned that I'm not sure if that's bad or not.

Ah, sorry, the underline problem. I did not connect that with your other comment, somehow. The underline will indeed end up in the hidden textarea. It might be possible to detect that multi-keystroke IME is happening by the way the text changes, and show an underline. I haven't looked into this yet since I'm also unsure how important this feature is and whether it justifies such a hack.
Comment 35 Anton Kovalyov (:anton) 2013-03-18 17:49:03 PDT
Created attachment 726440 [details] [diff] [review]
CodeMirror 3.1 as the default source editor

So during the workweek I continued hacking on our CM integration, upgraded CodeMirror to 3.1 and so on. There are still test failures and I probably won't get to this patch this week so uploading here just as a backup.
Comment 36 Anton Kovalyov (:anton) 2013-08-14 18:49:07 PDT
Created attachment 790588 [details] [diff] [review]
WIP 1

New effort, without backwards compatibility in code this time.
Comment 37 Anton Kovalyov (:anton) 2013-08-14 19:32:10 PDT
Created attachment 790597 [details] [diff] [review]
WIP 2
Comment 38 Anton Kovalyov (:anton) 2013-08-15 17:07:03 PDT
Created attachment 791033 [details] [diff] [review]
WIP 3
Comment 39 Brian Grinstead [:bgrins] 2013-08-20 14:12:29 PDT
I've been researching what changes would be required to add screen reader support to CodeMirror.  I have documented some of my findings here: http://bgrins.github.io/codemirror-accessible/.  I found a way to make selection movement work by updating the contents and selection of the focused textarea that CodeMirror is using behind the scenes.

I'm open to feedback about the implementation.  I'm particularly interested in feedback from someone who has accessibility experience and and who could comment on if it seems suitable (see the live demo on the project page).  I outline more details on the project page, and added a screencast with a quick demo.

Marijn,
The relevant changes to codemirror-accessible.js can be seen here: https://github.com/bgrins/codemirror-accessible/compare/df4df2366fe254f3ff528e27928af6eae6b9baf5...master#diff-17ed72d8617eb30a518f39b4e72a2033.  Basically, when the selection is collapsed, I temporarily set the textarea's value to the selected line and set the cursor location to the current selection location inside of extendSelection.  Then there is some code in readInput and resetInput to handle this state, and onKeyDown to reset the state.  This was arrived at through trial and error, and was the best way that I could get it to work with NVDA.  Maybe you can think of a better way, or some additional cases that I should test?  I think this could easily be hidden behind an option, by leaving cm.state.accessibleTextareaWaiting as always false if the option is false.

I also had a question about the inaccurateSelection tracking - is filling the input.value with cm.doc.getSelection() a performance problem? https://github.com/marijnh/CodeMirror/blob/612c6c9b2de394c14eb95459a5e11549912b12cd/lib/codemirror.js#L1493.  It is nice for the screen reader to actually select all of the text here so it can report the selection.  I have disabled the inaccurateSelection functionality in my tests, and 'select all' doesn't feel particularly slower, even with large inputs.
Comment 40 Anton Kovalyov (:anton) 2013-09-03 15:23:28 PDT
Splitting this bug into multiple smaller bugs. We decided to land CodeMirror integration piece-by-piece instead of one giant patch.
Comment 41 Paul Rouget [:paul] 2013-09-23 05:08:58 PDT
Raising the priority as this is blocking a feature of the app manager (bug 912891).
Comment 42 Anton Kovalyov (:anton) 2013-10-22 14:51:25 PDT
Adding the secreview? flag for Mark. CodeMirror (the actual library) was landed here: https://hg.mozilla.org/mozilla-central/rev/8db3d638ccd1
Comment 43 Mark Goodwin [:mgoodwin] 2013-11-14 02:25:11 PST
Security review completed: see bug 935694
Comment 44 Anton Kovalyov (:anton) 2013-11-26 14:36:19 PST
CodeMirror is now the default and only editor both in Nightly and Aurora.

Note You need to log in before you can comment on or make changes to this bug.