search

laurent22 / joplin

Joplin - the privacy-focused note taking app with sync capabilities for Windows, macOS, Linux, Android and iOS.
https://joplinapp.org
Other
45.62k stars 4.96k forks source link

Global search focuses text in notes so that edits overwrite highlighted text #6035

Closed msbentley closed 2 years ago

msbentley commented 2 years ago

I have noticed recently that when doing a global search, and choosing a note from the note list, the found text is highlighted in the note. So far so good. But when I click into the note (e.g. at the end to append something) the cursor jumps after the first character to the highlight and Joplin starts to over-write this text.

I've attached a short video to explain more clearly!

https://user-images.githubusercontent.com/1671065/149974279-7f5c02fd-0ed5-4ba4-87cc-c970c3d35d9a.mp4

Environment

Joplin version: 2.7.7 (although I think this happened on at least the few recent releases of the 2.7 series) Platform: linux OS specifics: ubuntu 20.04

Steps to reproduce

  1. Create a note with given text
  2. Search for this text in the global search
  3. Select the created note from the list
  4. Click at the end of the note to position the cursor to append some text
  5. Start tying
  6. Note that the first character is correctly placed, but that subsequent characters overwrite the "found" text in the note

(note that I have tested this in safe mode - as in the video - to make sure it was not due to a plugin)

Describe what you expected to happen

Regardless of the search highlighting, edits should occur where the cursor is positioned.

laurent22 commented 2 years ago

Is it really a new issue? I think it would have happened in previous versions too, because it's just the highlighted text being overwritten.

msbentley commented 2 years ago

Hmm, good question @laurent22 - I've only noticed in the last week or so, but perhaps I changed my workflow? I just can't imagine that I wouldn't have hit this earlier if it had always been there. Either way, I guess it's not expected behaviour?

tessus commented 2 years ago

This does not happpen in CodeMirror, so I suspect this is a WYSIWYG editor related issue. We should change the issue template to include the editor info when opening an editor bug.

msbentley commented 2 years ago

Just FWIW @tessus I'm not using the WYSIWYG editor (see video above). It may be that some specific cases are needed to reproduce the issue; I'll have a play and post back

tessus commented 2 years ago

Sorry, I was in the wrong branch. 2.6.9 does not have the issue. But I can reproduce it on 2.7.x (fdba6c076).

laurent22 commented 2 years ago

I guess it might be a new issue then, as it's true no-one ever mentioned this problem before and it probably would have came up. I don't know what could have caused this though.

tessus commented 2 years ago

We had a few editor related commits. I think 3 with sync scroll and 2 with search highlighting or something other related to search. I can't recall off the top of my head. P.S.: I might have labeled the PRs with editor.

splerman commented 2 years ago

I upgraded to 2.7.6 yesterday and 2.7.7 today (Windows...and not using the WYSWIG editor either) and can confirm the same behavior started for me on 2.7.6. Didn't report it because I hadn't had a chance to disable all plugins to see if it was a plugin causing the issue. Happy to provide any additional info/troubleshooting that might be useful.

laurent22 commented 2 years ago

Right, that's probably this commit:

https://github.com/laurent22/joplin/pull/5919/files

Before we didn't set the selection at all, and now we do. @ken1kob, do you think this issue can be fixed, or should we rollback for now? Perhaps if the goal is to focus the editor we could just call focus() on one of the container html elements? We did that for something else and it worked well.

godylockz commented 2 years ago

@laurent22 I suggest rolling this back, I think i've deleted countless things by just clicking the enter key after searching .... its preety bad. I reverted to 2.6.10 Unless your implementing a find/replace with the search functionality, it shouldn't be selecting for editing. Just for finding notes.

laurent22 commented 2 years ago

The feature has been reverted in c668bb03706bbf86c57c7152160c2a5ff59022e7 for now to give us time to consider how it should be fixed.

CalebJohn commented 2 years ago

Looks like the issue is a combination of #5919 and #5894. The bug itself is from #5894, which causes the selection to be remade every time the document is edited.

I'm going to follow up with a PR in a couple minutes.

ken1kob commented 2 years ago

@ken1kob, do you think this issue can be fixed, or should we rollback for now?

@laurent22, first, we have to clarify the spec of local/global search. For example:

Local search: (recently discussed in #5850)

Global search: (to be clarified here)

If this spec is OK, PR #6038 will fix this problem. (#6038 needs #5919.)

ken1kob commented 2 years ago

It would be better both #6037 and #6038 are applied.

msbentley commented 2 years ago

Just a heads-up - the behaviour is different, but not resolved in 2.7.8. Now the cursor position is not constantly reset, but the page seems to scroll either to the top (or first search match?). So no data loss is involved, but after each keystroke the page scrolls away from the cursor position:

https://user-images.githubusercontent.com/1671065/150221226-87ce8841-a11b-4b75-b130-dd69601d3c26.mp4