X Tutup
Skip to content

Do not redirect if browser asks for a .properties file#38181

Merged
mmattel merged 2 commits intomasterfrom
properties_htaccess
Dec 12, 2020
Merged

Do not redirect if browser asks for a .properties file#38181
mmattel merged 2 commits intomasterfrom
properties_htaccess

Conversation

@jvillafanez
Copy link
Member

@jvillafanez jvillafanez commented Dec 2, 2020

Description

files_pdfviewer app asks for a locale.properties file in order to handle the translations. In some setups, this request gets redirected to the files view, so the app can't get the translations.
This change makes possible to get the file so the files_pdfviewer can translate the pdf viewer content according to the language set by the user.

Related Issue

No issue related. Found in owncloud/files_pdfviewer#228 (comment)

Motivation and Context

The whole UI should be shown in the same language.

How Has This Been Tested?

  1. Install OC via docker setup (used official docker setup for 10.5)
  2. Install files_pdfviewer app
  3. As user1, change the ownCloud language settings for the user to anything other than English
  4. As user1, upload a pdf file
  5. As user1, access to that pdf file. The pdf viewer should open the file. The UI related to the pdf viewer should be translated

Screenshots (if appropriate):

Screenshot from 2020-12-02 10-50-15

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

@jvillafanez jvillafanez self-assigned this Dec 2, 2020
@update-docs
Copy link

update-docs bot commented Dec 2, 2020

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.

@jvillafanez
Copy link
Member Author

jvillafanez commented Dec 2, 2020

Couple of things to consider:

  • This should probably be included in 10.6
  • The change will likely affect only to new installations. Updated installations might need to manually adjust this, or we might need to provide a migration / repair step

@jvillafanez jvillafanez requested a review from micbar December 2, 2020 09:59
@mmattel
Copy link
Contributor

mmattel commented Dec 2, 2020

Updated installations might need to manually adjust this, or we might need to provide a migration / repair step

I would not recommend a manual driven adoption for existing installations. How ill you propagate this properly so it will be implemented? Just giving a note in the changelog and hope is imho insufficient...
(But thanks for raising that point.)

@jvillafanez
Copy link
Member Author

jvillafanez commented Dec 2, 2020

The touched function is called during the upgrade, and there is also an occ command to update the .htaccess manually if needed.
QA might still need to verify it, but I don't think we need to touch additional things.

VicDeo
VicDeo approved these changes Dec 4, 2020
View reviewed changes
Copy link
Member

@VicDeo VicDeo left a comment

Choose a reason for hiding this comment

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

Makes sense

@mmattel
Copy link
Contributor

mmattel commented Dec 4, 2020

You should add a changelog to satisfy the bot 😄
This is maybe a good place to put a note to use the occ command for existing installations

@jvillafanez
Copy link
Member Author

jvillafanez commented Dec 9, 2020

I haven't seen a complain about this, so I don't think it's critical enough to make noise.
Existing installation can either update to pick this change (when it's released), or apply the patch. In the second case, they should know what they're doing anyway and what are they applying, otherwise they should wait until a new version with the fix is released.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 9, 2020

Kudos, SonarCloud 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
No Duplication information No Duplication information

@mmattel
Copy link
Contributor

mmattel commented Dec 9, 2020

All green, merging?

@mmattel mmattel merged commit 41452f7 into master Dec 12, 2020
@delete-merged-branch delete-merged-branch bot deleted the properties_htaccess branch December 12, 2020 10:09
This was referenced Mar 16, 2021
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@VicDeo VicDeo VicDeo approved these changes

@DeepDiver1975 DeepDiver1975 Awaiting requested review from DeepDiver1975

@micbar micbar Awaiting requested review from micbar

Assignees

@jvillafanez jvillafanez

Labels

3 - To Review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jvillafanez @mmattel @VicDeo
X Tutup