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

Spellchecker doesn't work with new editor #4977

Closed
comradekingu opened this issue Dec 4, 2020 · 15 comments · Fixed by #5158
Closed

Spellchecker doesn't work with new editor #4977

comradekingu opened this issue Dec 4, 2020 · 15 comments · Fixed by #5158
Assignees
Labels
bug Something is broken. help wanted Extra attention is needed. ux Issues related to user experience.
Milestone

Comments

@comradekingu
Copy link
Contributor

comradekingu commented Dec 4, 2020

<textarea style="position: absolute; bottom: -1em; padding: 0px; width: 1px; height: 1em; outline: currentcolor none medium;" autocorrect="off" autocapitalize="off" spellcheck="false" tabindex="260" wrap="off"></textarea>

Tried setting autocorrect="on", but then I couldn't edit the field at all.
Works elsewhere in Weblate, like the "Search…" in https://hosted.weblate.org/accounts/profile/#notifications
which is just <input class="search-input" type="text" placeholder="Search…">

FF 78.5.0esr (64-bit) on Linux with the standard dictionaries. Does this happen to anyone else?

Edit: No luck in Vivaldi either.

@nijel
Copy link
Member

nijel commented Dec 4, 2020

Probably yes because what you see is not actually a textarea, but styled content and the actual textarea is hidden. We're now using CodeMirror for editing text to support highting placeables, double space or syntax for some markup formats.

@comradekingu
Copy link
Contributor Author

comradekingu commented Dec 6, 2020

Can it be applied by Weblate then?

@Fat-Zer
Copy link
Collaborator

Fat-Zer commented Dec 6, 2020

It's quite a major drawback... At least for me, as I don't have innate spelling skills, so I have to copy every other phrase to another tab and it's damn annoying...

I don't need a fancy WYSIWYG edit field, is there an option to revert this to a plain text editor?

@Fat-Zer
Copy link
Collaborator

Fat-Zer commented Dec 6, 2020

PS: also it breaks pasting with middle button and copy to X.org's select buffer... It's probably caused by the same thing, but should I report it as a yet another bug?

@nijel
Copy link
Member

nijel commented Dec 6, 2020

Pasting with middle button should work (it works for me), but the X selection indeed does not work. Feel free to report this at https://github.com/codemirror/CodeMirror/issues (assuming you can reproduce it on their demo as well - https://codemirror.net/index.html).

As for spell checking, I agree that it's a major drawback. We will probably end up having own JS implementation.

On the other side we need to move towards more comfortable editor and that's not possible with simple textarea. The CodeMirror editor has been in place for comments for several releases and I've seen no complaints on that. If you know some editor which will not break browser spellcheck and allows highlighting of some strings, we are open to suggestions.

@Fat-Zer
Copy link
Collaborator

Fat-Zer commented Dec 7, 2020

I'm not insisting, but still believe it would be nice to have an option to disable the CodeMirror and revert back to the plain old textarea as a workaround for the issue. I'm not a web developer, but I suppose it shouldn't be hard. I've just issued $('.CodeMirror')[0].CodeMirror.toTextArea(); from the web console and it worked quite fine. The buttons to insert special characters on the toolbar malfunctioned and the char counter didn't worked but in general it went pretty smooth...

I can't suggest another editor, but I suppose that anything fancier than a textarea will prevent the use of the in-browser spell checker. I assume you may build in the spell checker into java script, but it may be tricky to guess the languages.

@nijel, thanks, those issues about selection buffer look like solely a firefox-related trouble (at least I can't reproduce them on chrome). I would rather not report them to CodeMirror upstream yet as far as I still use a rather old version of the browser (68.4.1) and it'd be not that easy for me to upgrade right now...
PS: About pasting with the middle button: if middlemouse.contentLoadURL=true in about:config, pasting a URL in a such way triggers a redirect. Yet that works fine for a normal textarea. I haven't tested this thoroughly on a first glance...
PPS: sorry for a bit of unrelated noise.

@nijel
Copy link
Member

nijel commented Dec 7, 2020

I'm not insisting, but still believe it would be nice to have an option to disable the CodeMirror and revert back to the plain old textarea as a workaround for the issue.

I'd rather prefer a single solution that would work for all. Having this configurable will introduce two code paths for any client-side manipulation with the editor and it will break things over the time.

it may be tricky to guess the languages.

We don't have to guess languages, we know into which language is user translating.

I can't suggest another editor, but I suppose that anything fancier than a textarea will prevent the use of the in-browser spell checker.

That doesn't seem to be the case. There seems to be solutions which do support highlighting and still are real textarea, for example https://github.com/kueblc/LDT. The question is whether it doesn't break other things (RTL support is usually problematic) and whether it's maintained in a way we could rely on it...

@nijel
Copy link
Member

nijel commented Dec 7, 2020

I've quickly tested LDT and it breaks with RTL and line wrapping seems to be messed up as well, so that one doesn't seem to be viable solution...

@nijel nijel added help wanted Extra attention is needed. ux Issues related to user experience. labels Dec 7, 2020
@nijel
Copy link
Member

nijel commented Dec 7, 2020

The question is whether we can use some existing solution or reinvent the wheel and write something that works for us. There seem to be two approaches:

Display overlay on top of textarea

Uses native textarea editing and displays overlay with the result of highlighting. The user is still editing the actual textarea, but he sees something with the highlighting. The downside is that formatting cannot change the size of the text because it has to match to the textarea. We currently rely on this to display placeables, but it could be adjusted. Most of the editors also break on line wrapping - textarea does that, the overlay has to match, what doesn't work with LDT.

Some useful links:

This suffers issue of using browser edit history, for which there doesn't seem to be a solution for Firefox, see https://bugzilla.mozilla.org/show_bug.cgi?id=1220696

Using contenteditable

This is mostly for WYSIWIG, but we really do not need that. On the other side it's closer to what we are looking for - freely stylable editor. It will be probably hard to find solution that will work for us, as most the solutions seems to either focus on code editing (which will probably have troubles with RTL) or being WYSIWIG HTML editors. The tricky part here is also handling undo/redo as built-in gets broken on DOM manipulation (which are probably necessary for highlighting).

What we are looking for

  • Syntax highlighting editor
  • Utilize spell checking from the browser
  • Proper undo/redo controls from browser
  • Seamless support for RTL languages
  • Line wrapping
  • Text completion (currently used for @username mentions, but might be extended)
  • We probably do not need optimization tricks as the edited text is typically quite short
  • Custom highlighting rules (to handle Weblate features like double space or placeables)
  • Markup highlighting for HTML, Markdown and reStructuredText
  • No auto indentation
  • Native browser tab behaviour
  • Automatic sizing

@nijel
Copy link
Member

nijel commented Dec 8, 2020

Doing some research on this and here are my findings:

  • CodeMirror can only use own spell checker, see Does not integrate with Chrome's spell check  codemirror/codemirror5#1017. That is annoying as we would have to distribute all the dictionaries and it would be still broken with third-party browser extensions such as LanguageTool.
  • Using transparent <textarea> and highlighting using rendered side element seems doable approach, but it limits highlighting capabilities (cannot use anything what would break alignment of overlaying elements).
  • Using contenteditable would need quite some code to handle things textarea does natively - forcing plain text on paste, undo/redo implementation (because we will be changing DOM for syntax highlighting).

I will try to dig deeper into existing solutions for highlighting <textarea> and see if some of that matches our needs.

@nijel nijel added the bug Something is broken. label Dec 14, 2020
@nijel nijel added this to the 4.4.1 milestone Dec 14, 2020
nijel added a commit to nijel/weblate that referenced this issue Jan 7, 2021
It has proven to be a wrong decision as that disables in-browser spell
checker.

This reverts the following commits:

* 123340c
* d2e3eef
* 96a7d55
* 6c31d13
* cecbf9c
* fda647c
* 39ed573
* 017d7a8
* f107cdd
* 9a26161
* 4b2197e
* c69238f
* 16a9f87

Fixes WeblateOrg#4977
Fixes WeblateOrg#5114
Fixes WeblateOrg#5049
nijel added a commit to nijel/weblate that referenced this issue Jan 7, 2021
@kodiakhq kodiakhq bot closed this as completed in #5158 Jan 7, 2021
kodiakhq bot pushed a commit that referenced this issue Jan 7, 2021
It has proven to be a wrong decision as that disables in-browser spell
checker.

This reverts the following commits:

* 123340c
* d2e3eef
* 96a7d55
* 6c31d13
* cecbf9c
* fda647c
* 39ed573
* 017d7a8
* f107cdd
* 9a26161
* 4b2197e
* c69238f
* 16a9f87

Fixes #4977
Fixes #5114
Fixes #5049
kodiakhq bot pushed a commit that referenced this issue Jan 7, 2021
@github-actions
Copy link

github-actions bot commented Jan 7, 2021

Thank you for your report, the issue you have reported has just been fixed.

  • In case you see a problem with the fix, please comment on this issue.
  • In case you see a similar problem, please open a separate issue.
  • If you are happy with the outcome, don’t hesitate to support Weblate by making a donation.

@nijel
Copy link
Member

nijel commented Jan 7, 2021

The fix is now live at https://hosted.weblate.org/, it would be great if you could provide feedback how it works because it is completely different implementation than before and I expect that I've missed something during testing and some bugs will appear.

@nijel nijel self-assigned this Jan 7, 2021
@Fat-Zer
Copy link
Collaborator

Fat-Zer commented Jan 7, 2021

Looks like it works pretty well. As for now I can spot a couple of minor regression:

  • The references and verbatim text highlighting doesn't work for the ReST (if they are supposed to).
  • There are some issues with positioning of cursor when there is italic text in the line. But I suppose it would be a tradeoff of the method.
  • Verbatim highlight in markdown doesn't work too. [added +6min]
  • The cursor appears to be mispositioned if a line is wrapped on a double space [added +24min]

@nijel
Copy link
Member

nijel commented Jan 7, 2021

Some CSS is still missing for some highlighting, I need to tweak that.

nijel added a commit that referenced this issue Jan 8, 2021
It is now based Coy theme from Prismjs, so it covers all tokens. We do
not want to use the full theme as we only want to apply colors here.

Issue #4977
@nijel
Copy link
Member

nijel commented Jan 8, 2021

I've just deployed additional fixes. Feel free to open a separate issue in case something is still broken (so that it is not lost here).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken. help wanted Extra attention is needed. ux Issues related to user experience.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants