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

Fix opening editors in new tab #4500

Merged
merged 2 commits into from
Dec 15, 2020
Merged

Fix opening editors in new tab #4500

merged 2 commits into from
Dec 15, 2020

Conversation

LukasHirt
Copy link
Collaborator

When opening the editors view in a new tab, we were using the origin of location. This would break in case we have Web deployed to a different path than root e.g. http://owncloud/apps/web.

@LukasHirt LukasHirt requested a review from fschade December 15, 2020 09:38
@LukasHirt LukasHirt self-assigned this Dec 15, 2020
Copy link
Contributor

@fschade fschade left a comment

Choose a reason for hiding this comment

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

💯

@phil-davis
Copy link
Contributor

phil-davis commented Dec 15, 2020

https://drone.owncloud.com/owncloud/web/12278/52/11
@LukasHirt various test scenarios that open stuff in media viewer are failing.
Without having tried it, my first guess is that the files get opened in a new tab, and so the test code does not see them. But I don't see anything in the diff that should change the behaviour to now open in a new tab by default or...

@LukasHirt
Copy link
Collaborator Author

@phil-davis This was actually because of an incorrect route name. We dropped the automatic prefixing with app id in the file actions because some route names are not doing that but I forgot to fix the route in media viewer. Already pushed fixed.

@LukasHirt LukasHirt merged commit 4029522 into master Dec 15, 2020
@LukasHirt LukasHirt deleted the fix-editors-new-tab branch December 15, 2020 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants