Skip to content

Stop encoding sidebar path #1470

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Nov 18, 2022
Merged

Stop encoding sidebar path #1470

merged 2 commits into from
Nov 18, 2022

Conversation

@skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Nov 18, 2022 •
edited
Loading

Fix #1465

Let's see if other areas exploded because of this

References:

viewer/src/views/Viewer.vue

Lines 907 to 914 in a1e581d

async showSidebar() {
// Open the sidebar sharing tab
// TODO: also hide figure, needs a proper method for it in server Sidebar
if (OCA?.Files?.Sidebar) {
await OCA.Files.Sidebar.open(this.sidebarOpenFilePath)
}
},

viewer/src/views/Viewer.vue

Lines 293 to 295 in a1e581d

sidebarOpenFilePath() {
return this.currentFile?.davPath?.split(getUserRoot())[1]
},

https://github.com/nextcloud/server/blob/b9bbb25771fc704d0f5a9de7bc2ae0cae73cf81b/apps/files/src/views/Sidebar.vue#L155

@skjnldsv skjnldsv added the 3. to review Waiting for reviews label Nov 18, 2022
@skjnldsv skjnldsv self-assigned this Nov 18, 2022
@skjnldsv

This comment was marked as resolved.

Sign in to view
@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Nov 18, 2022
@skjnldsv skjnldsv force-pushed the fix/sidebar-encoding branch from 66ffcec to bdb46cf Compare November 18, 2022 08:56
@skjnldsv skjnldsv added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Nov 18, 2022
@skjnldsv skjnldsv changed the title Stop encoding path Stop encoding sidebar path Nov 18, 2022
@skjnldsv
Copy link
Member Author

/backport to stable25

@backportbot-nextcloud backportbot-nextcloud bot added the backport-request Pending backport by the backport-bot label Nov 18, 2022
@skjnldsv
Copy link
Member Author

Done, added cypress tests too

beardhatcode
beardhatcode approved these changes Nov 18, 2022
View reviewed changes
Copy link
Contributor

@beardhatcode beardhatcode left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good to me, but I do not have time to check if it actually works 😅 .
Just a reminder to also check if PDF's can still be opened (It should but it would be good to validate), the alste time I messed with paths here, it broke the PDF viewer for some users.

@skjnldsv
Copy link
Member Author

Seems like something is off anyway xD
Let me fix cypress

@skjnldsv
Stop encoding sidebar path
0677ed8 
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@skjnldsv
Adjust testing
4c80e07 
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@skjnldsv skjnldsv force-pushed the fix/sidebar-encoding branch from bdb46cf to 4c80e07 Compare November 18, 2022 11:40
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Nov 18, 2022
@skjnldsv skjnldsv merged commit 84a2089 into master Nov 18, 2022
@skjnldsv skjnldsv deleted the fix/sidebar-encoding branch November 18, 2022 11:46
@skjnldsv
Copy link
Member Author

Will the backport work? 🙏

@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Nov 18, 2022
Merged
1 task
@backportbot-nextcloud backportbot-nextcloud bot removed the backport-request Pending backport by the backport-bot label Nov 18, 2022
@skjnldsv
Copy link
Member Author

It's a miracle! 🎄

@juliusknorr juliusknorr mentioned this pull request Nov 28, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@beardhatcode beardhatcode beardhatcode approved these changes

@juliusknorr juliusknorr juliusknorr approved these changes

Assignees

@skjnldsv skjnldsv

Labels

4. to release Ready to be released and/or waiting for tests to finish

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Broken details sidebar due to double URL encoded path

4 participants

@skjnldsv @beardhatcode @juliusknorr