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

Albums: Replace ASCII double quotes in titles with Unicode quotes instead of single quotes #2891

Closed
dror3go opened this issue Nov 14, 2022 · 12 comments
Assignees
Labels
docs 📚 Write, improve, or review documentation idea Feedback wanted / feature request released Available in the stable release

Comments

@dror3go
Copy link

dror3go commented Nov 14, 2022

Following #1196 where we enabled keywords with single/double quotation marks, PhotoPrism currently converts double quotation marks in album titles with single quotation marks.
Though not a high priority bug, it feels weird for users in languages which make use of double quotation marks for acronyms.

@dror3go dror3go added the bug Something isn't working label Nov 14, 2022
@lastzero
Copy link
Member

Think that was a precaution measure in the aftermath of the log4j debacle. Double quotes would also simplify code injection and may not be supported on many filesystems in case you want to use album names for zip files or as folder names. As such, it's a decision we made (for now) and not a bug. Feel free to help with improving our docs so that it does not look as that happened by mistake and should be reported as a bug.

@lastzero lastzero added idea Feedback wanted / feature request docs 📚 Write, improve, or review documentation and removed bug Something isn't working labels Nov 14, 2022
@dror3go
Copy link
Author

dror3go commented Nov 27, 2022

While I understand the technical rationale - as an end user I still find it to be a bug.
Though not a German speaker myself, I think it's a little bit like if German letters such as Ä and Ö would've been replaced with A and O.
As I mentioned in the original post - I think it's not a high priority issue, but still an issue nonetheless. I feel that labeling it as documentation issue is saying it's WONTFIX and I should expect this behavior to remain as-is.

Adding a note on the docs wouldn't improve anything from my perspective - I just don't think that an end user should be expected to read the docs just like I'm not reading manual guides of a new TV for example. Ideally things should work "as expected".

@lastzero
Copy link
Member

Would have been replaced? Where? How?

Screenshot_20221127_203940

@dror3go
Copy link
Author

dror3go commented Nov 27, 2022

They're not, that's my point.
As an end user - would you think it was a bug if PhotoPrism replaced the first two letters to "OA"?

@lastzero
Copy link
Member

בּד״א
works as well
״ is not the same as "

@lastzero
Copy link
Member

All of the examples I found on https://www.myjewishlearning.com/article/a-guide-to-jewish-acronyms-and-abbreviations/ can be used as album names, just not the ASCII " which would require escaping and a lot of security checks and which technically is not the right character for RTL languages either as it is LTR.

@lastzero
Copy link
Member

Would you be happy if we convert " to a proper Unicode character?

lastzero added a commit that referenced this issue Nov 28, 2022
Signed-off-by: Michael Mayer <michael@photoprism.app>
@lastzero lastzero changed the title Albums: Double quotation marks are being replaced with single quotation marks in album titles Albums: Replace ASCII double quotes in titles with Unicode quotes instead of single quotes Nov 28, 2022
@lastzero lastzero added the please-test Ready for acceptance test label Nov 28, 2022
@lastzero lastzero self-assigned this Nov 28, 2022
@lastzero
Copy link
Member

I have started a development preview build for testing. It should be ready within an hour. You can then also use our demo to test it.

@lastzero
Copy link
Member

lastzero commented Nov 28, 2022

@dror3go
Copy link
Author

dror3go commented Nov 28, 2022

בּד״א works as well ״ is not the same as "

Yes, I'm well aware :)

Thing is that de-facto nobody other than Apple devices is using this character, since it's not exposed to users on physical and non-physical keyboards.
Even worse - Apple automatically converts " to ״ even when it's clearly wrong, like in code-related things. Imagine you send your teammate a snippet of a log which includes strings with quotes - your teammate will receive a broken text which can't be copy-pasted to dev-related tools. The best example would be a JSON object.

I personally prefers these quotes over a single quote, but would like to stress that a search for an album with straight quotes must include in the results albums with curly quotes in their names.

@lastzero
Copy link
Member

lastzero commented Nov 28, 2022

Thank you for your feedback! Much appreciated. If you look at our search filter overview, you'll notice that we use " to quote strings with spaces:

So to search for " you would have to escape it in some way, which many users probably wouldn't do (instrictively). If you use the character for file and folder names, you would also have to replace or escape it somehow. Thus we tried to avoid it as much as possible.

Please note that Unicode search, normalization, and comparison are generally complex topics. While I'm happy to provide you with what I hope is a quick solution, we can't make this a full-time project in the next few weeks to cover all the special cases if the underlying database doesn't already cover them (which needs to be tested before we draw conclusions and develop new functionality).

@lastzero
Copy link
Member

A good starting point to learn more about Unicode equivalence in search is the Wikipedia article:

@graciousgrey graciousgrey added tested Changes have been tested successfully and removed please-test Ready for acceptance test labels Apr 24, 2023
@graciousgrey graciousgrey added released Available in the stable release and removed tested Changes have been tested successfully labels May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs 📚 Write, improve, or review documentation idea Feedback wanted / feature request released Available in the stable release
Projects
Status: Release 🌈
Development

No branches or pull requests

3 participants