X Tutup
Skip to content

Fix the preview of text files containing BOM for utf8#39669

Merged
jvillafanez merged 3 commits intomasterfrom
fix_bom_preview
Feb 15, 2022
Merged

Fix the preview of text files containing BOM for utf8#39669
jvillafanez merged 3 commits intomasterfrom
fix_bom_preview

Conversation

@jvillafanez
Copy link
Member

@jvillafanez jvillafanez commented Jan 10, 2022

Adjusted range so the BOM isn't detected as an arabic char.
Latin is also added as part of possible fonts, not just if there isn't
any match

Description

BOM was detected as arabic char. Alternative fix to #39644

Related Issue

#39645

Motivation and Context

How Has This Been Tested?

  1. Create a text file containing the BOM and some latin chars
  2. Upload the file to ownCloud
  3. The preview should show correctly.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

Adjusted range so the BOM isn't detected as an arabic char.
Latin is also added as part of possible fonts, not just if there isn't
any match
@update-docs
Copy link

update-docs bot commented Jan 10, 2022

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

['range' => [0xfb1d, 0xfb4f], 'script' => 'Hebrew'], // some unicode chars aren't assigned
['range' => [0xfb50, 0xfdff], 'script' => 'Arabic'],
['range' => [0xfe70, 0xfeff], 'script' => 'Arabic'],
['range' => [0xfe70, 0xfefc], 'script' => 'Arabic'],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexAndBear
Copy link

We should check manually if the code now works with all BOMs 0xEF,0xBB,0xBF

@jvillafanez
Copy link
Member Author

Added unit tests to verify the text analyzer process the BOM correctly.
I've also checked manually that a file with latin chars and a BOM marker at the beginning is previewed properly

@AlexAndBear AlexAndBear self-requested a review January 14, 2022 14:01
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jvillafanez jvillafanez merged commit f42653c into master Feb 15, 2022
@delete-merged-branch delete-merged-branch bot deleted the fix_bom_preview branch February 15, 2022 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

X Tutup