-
Notifications
You must be signed in to change notification settings - Fork 114
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
cache clipboard image to prevent duplicating in anki media #776
cache clipboard image to prevent duplicating in anki media #776
Conversation
✔️ No visual differences introduced by this PR. View Playwright Report (note: open the "playwright-report" artifact) |
ext/js/background/backend.js
Outdated
* @returns {string} | ||
*/ | ||
_generateAnkiNoteMediaFileName(prefix, extension, timestamp, definitionDetails) { | ||
_generateAnkiNoteMediaFileName(prefix, extension, timestamp, definitionDetails = null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accepting null here (and actually using it) means that different definitions could result with the same filename if they are looked up in the same second, right? Can this cause problems?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hadn't considered that. If someone did
- copy image A
- add term A
- copy image B
- add term B
and 2-4 happened within the same second, image B would unexpectedly overwrite image A in anki's media folder.
It's pretty hard to do it that fast, but a couple ways to prevent it:
- Add more digits to the timestamp
- Generate a UID and either append it, or replace the timestamp
- Use deleteExisting: false in the ankiconnect call. Anki would then not overwrite files on filename. Would require another AnkiConnect call to check if a file is already added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I guess using a more precise timestamp is the simplest solution.
BTW, is there any point in keeping the definitionDetails input at all? Wouldn't it be simpler to just eliminate it instead of creating a branch in the logic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I guess with the more precise timestamp, the term/reading isn't needed in any of the filenames.
Resolves #767
Since the same filename would now be used on different cards it shouldn't contain the term/reading, as that would mess with searching in anki.