Closed A-312 closed 4 years ago
marijnh
commented
4 years ago Works for me in Chrome and Firefox (but since the code you show doesn't call setCursor, I can't be sure that I'm doing the same things that you're doing).
A-312
commented
4 years ago Works for me in Chrome and Firefox (but since the code you show doesn't call
setCursor, I can't be sure that I'm doing the same things that you're doing).
In your webconsole, when you try .setCursor, the cursor will don't move
A-312
commented
4 years ago <article>
<h2>Native Spellchecker Demo</h2>
<p>Codemirror uses <code>Codemirror.inputStyle: `contenteditable`</code> with <code>spellcheck: true<code>.</p>
<form><textarea id="code" name="code">
Native abc tesst
</textarea></form>
<form><textarea id="code2" name="code2">
No abc, textareaInput
</textarea></form>
<script>
var editor = CodeMirror.fromTextArea(document.getElementById("code"), {
inputStyle: "contenteditable",
spellcheck: true,
});
var editor2 = CodeMirror.fromTextArea(document.getElementById("code2"), {});
</script>
</article>editor.setSelection({line:0, ch:2}, {line:0, ch:5}) doesn't work :
editor2.setSelection({line:0, ch:2}, {line:0, ch:5}) works :
A-312
commented
4 years ago In dom.js, I add log and trace in range function because it calls the native/browser function to selection the text, I got (line 40):
if (document.createRange) range = function(node, start, end, endNode) {
console.log("RANGE:")
console.log(node, start, end, endNode)
console.trace()
let r = document.createRange()
r.setEnd(endNode || node, end)
r.setStart(node, start)
return r
}editor.setSelection(...)Args = {line:0, ch:2}, {line:0, ch:5}
Output RANGE: "Native" 5 6 undefined
editor2.setSelection(...)Args = {line:0, ch:2}, {line:0, ch:5}
range function in dom.js is called 3 times with editor.setSelection(...)
A-312
commented
4 years ago Depending of where is the cursor when I do editor.setSelection({line:0, ch:2}, {line:0, ch:5}) and if I do editor.focus() the selection can work:
editor.setSelection will do nothing.editor.setSelection will work if I use editor.focus() after.I try to set the focus out with code: document.getElementsByTagName('A')[0].focus() and use editor.setSelection after, this will don't work.
If my editor (CM) does not have the focus,
editor.setSelectionwill work if I useeditor.focus()after.
editor.setSelection({line:0, ch:2}, {line:0, ch:5})
Output RANGE: "Native" 5 6 undefined
editor.focus()
Output RANGE: "Native" 2 5 "Native"
A-312
commented
4 years ago .focus() workWhen I comment if (!this.selectionInEditor()), focus will refresh selection each time and getSelection on 1. or 2. will work if I do .focus() after.
Now, I think I know why editor2.getSelection work and not editor.getSelection, it seem showSelection is never called with editor.getSelection.
Ok...
This is the render loop: https://github.com/codemirror/CodeMirror/blob/28568ba037146b2d13857849650db94b8ec09385/src/display/operations.js#L56-L68
showSelection (I mean cm.display.input.showSelection to his closest friends) is called in endOperation_W2 if op.selectionChanged is true.
In endOperation_R2 function :
op.selectionChanged is defined in startOperation:
I have to understand why startOperation is not called or why startOperation is called with selectionChanged is false.
Now I understood, i can make a hack (temporary solution) for my code:
I find:
editor.setSelection({line:0, ch:2}, {line:0, ch:5})
editor.display.input.showSelection(editor.display.input.prepareSelection(), true)I have to understand why
startOperationis not called or whystartOperationis called withselectionChangedisfalse.
Ok... I did a mistake, I forget to watch the value of selectionChanged (I thought I did it).
showSelectionshowSelection doesn't work when takeFocus is false. đ
editor.setSelection({line:0, ch:2}, {line:0, ch:5}) works fine if I replace takeFocus by true.
Caused by: https://github.com/codemirror/CodeMirror/commit/b9e0c4cd0bab86a440cb10e3505bd6691bb0f001 linked to #3979 in #9 (https://github.com/notepadqq/CodeMirror/pull/9/commits/b9e0c4cd0bab86a440cb10e3505bd6691bb0f001)
A-312
commented
4 years ago The condition op.focus == activeElt() seems that it will be always false because op.focus is boolean and activeElt() returns an HTMLElement.
HTMLElement == true
false
HTMLElement == false
false
document.body == true
false
document.body == false
falseactiveElt() always return an HTMLElement.
https://github.com/codemirror/CodeMirror/blob/28568ba037146b2d13857849650db94b8ec09385/src/util/dom.js#L67-L80
I think this code is not necessary, because it is already tested in L110 in showSelection:
marijnh
commented
4 years ago Ah, right, the issue only occurs when the editor has focus within the window, but the window doesn't itself have focus.
I looked at your patch but I think there's a much less invasive way to fix this. See attached patch. Does that work for you?
A-312
commented
4 years ago My change is invasive but needed and it doesn't make depreciation and don't not changz the behavior.
I need to test but in first looking (i just reviewed not test), that doesn't sound perfect with no-focus. Without calling showPrimarySelection the selection will be not processing because the selection can appear without focus.
marijnh
commented
4 years ago I'd prefer that you actually test. It can be painful to give up one's patch, but I really much prefer the low-impact version of the fix.
A-312
commented
4 years ago I try with:
This editor uses Codemirror.inputStyle: 'contenteditable' and spellcheck: true.
The native spellchecker (of the browser) does not run with Codemirror.inputStyle: 'textarea' (default value).
Like I say, in contenteditableInput the selection appear only when the textarea have focus but in textareaInput the selection always appear (with or without focus). I think you would the same behavior depending of the Style.
A-312
commented
4 years ago (I got a bug with editor2.focus(), it seems to not work, I mean I can't write directly after I got the focus with this function in editor2)
marijnh
commented
4 years ago Like I say, in contenteditableInput the selection appear only when the textarea have focus
That's just how the native selection works, so it is kind of implied in the decision to use the native selection.
A-312
commented
4 years ago Can you publish your change on npm ?
marijnh
commented
4 years ago CodeMirror uses a monthly release cycle. There'll be a new release around the 20th of the month.
A-312
commented
4 years ago @marijnh I tried your code on master of Easymde: My change works fine, your change seem to doesn't work, I let you try:
I add a color change, to show you that it is been the last version of codemirror in git (not the old one).
A-312
commented
4 years ago On my PR, i did 4 commits :
Each commit is one step, you can try each commit step by step.
A-312
commented
4 years ago (If my change need to be enabled with option tell me I will make this changes).
marijnh
commented
4 years ago It is only code review to delete unused code.
That isn't actually unused code. The focus field in an operation will hold false or a DOM node.
Your fix seems to unconditionally mess with the DOM selection, which is not a good idea (when the user doesn't have the editor focused, but for some reason its content is changed, that'll disrupt the selection).
So, again, I don't want to merge these changes.
Running just CodeMirror, without any external code, I can really verify that the problem is gone with my patch. If can set up a demo without your codebase that still shows the problem, let me know, but for now I'm closing this.
A-312
commented
4 years ago Running just CodeMirror, without any external code, I can really verify that the problem is gone with my patch. If can set up a demo without your codebase that still shows the problem, let me know, but for now I'm closing this.
@marijnh Please, I haven't finished yet, can you reopen ? I can prove it without easymde (I wanted to save some time with remaking an existing script). I will not speak again of easymde, here.
Why am I sure?
Without calling showPrimarySelection the selection will be not processing because the selection can appear without focus.
My example with only codemirror:
var editor = CodeMirror.fromTextArea(document.getElementById('code'), {
inputStyle: 'contenteditable',
spellcheck: true
})
document.getElementById('icanproveit').addEventListener('click', function () {
const cm = editor;
const startPoint = cm.getCursor('start');
const endPoint = cm.getCursor('end');
cm.replaceSelection('**' + cm.getSelection() + '**');
startPoint.ch += 2;
endPoint.ch += 2;
cm.setSelection(startPoint, endPoint)
cm.focus();
})My full page to test:
This editor uses Codemirror.inputStyle: 'contenteditable' and spellcheck: true.
The native spellchecker (of the browser) does not run with Codemirror.inputStyle: 'textarea' (default value).
Result:
marijnh
commented
4 years ago I seeâthat's a different issue from the one before, though. Attached patch seems to help with that.
A-312
commented
4 years ago This work fine thanks you!
Hi,
When I try to do
editor.setCursor({line: 0, ch: 2})withinputStyle: "contenteditable",setting, the cursor doesn't change this position but this work fine withinputStyle: "editor",.I'm looking to make a fix/pr, if you have advice about fixing this issue, please feel free to comment.
Example with demo/abc.html :
(External linked issue : https://github.com/Ionaru/easy-markdown-editor/issues/160 & https://github.com/zestedesavoir/zds-site/issues/5652)