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

Files sometimes do not get opened immediately on internal links #3815

Open
juliushaertl opened this issue Jul 9, 2024 · 7 comments
Open
Labels
1. to develop Waiting for a developer bug Something isn't working

Comments

@juliushaertl
Copy link
Member

juliushaertl commented Jul 9, 2024

Steps to reproduce

  1. Have an internal link like /f/1234
  2. Open it in a browser (e.g. from clicking in an email)

Expected behaviour

The file should open

Actual behaviour

Sometimes the file doesn't open automatically and there is an error thrown. It seems happening more often with cleared browser cache.

Now the odd thing here is that the error is thrown before the viewer app is initialized, so there seems to be some kind of timing issue when trying to get the file actions for the requested file.

nextcloud/server#45586 would help with the error but I cannot see how it would solve the file then just not opening up.

I have not managed to reproduce this locally on any of the versions, but tech-preview is affected (currently on 29.0.0) and another instance on 28.0.6 (both should be upgraded 🙈 )

@susnux Do you have any idea on why the order in which the handleOpenFile and the viewer registrations would be executed might differ?

Screenshot 2024-07-09 at 10 05 21 Screenshot 2024-07-09 at 10 06 15
@juliushaertl juliushaertl added bug Something isn't working 1. to develop Waiting for a developer labels Jul 9, 2024
@juliushaertl
Copy link
Member Author

juliushaertl commented Jul 9, 2024

Upgraded the 29 instance to 29.0.3, still reproducible sometimes, but now without an error

Edit: Still one for the sidebar:

Screenshot 2024-07-09 at 10 47 04

@pedropintosilva
Copy link
Contributor

cc'ing as requested: @mmeeks @karlitschek

@bentuna
Copy link

bentuna commented Aug 1, 2024

We are running 28.0.8 and can still confirm the issue is happening sometimes.

@juliushaertl
Copy link
Member Author

I had a chat with @susnux about this and this seems to be a design flaw of the viewer api registering the file type handlers too late. He will think about a possible workaround.

@juliushaertl
Copy link
Member Author

@susnux I looked a bit further into it while fixing a regression with 30 nextcloud/server#47014

I still have not managed to trigger this race condition locally. I tried to think about a way to fake this timing issue but cannot figure out how I could manually delay the viewer registration like it is happening on those affected systems. But thought about a workaround like this, kind of deferring the handleOpenFile until the DOMContentLoaded event from viewer has registered the file actions.

diff --git a/apps/files/src/components/FilesListVirtual.vue b/apps/files/src/components/FilesListVirtual.vue
index 233353d0985..62cc53fd093 100644
--- a/apps/files/src/components/FilesListVirtual.vue
+++ b/apps/files/src/components/FilesListVirtual.vue
@@ -209,7 +209,7 @@ export default defineComponent({
                const { id } = loadState<{ id?: number }>('files', 'fileInfo', {})
                this.scrollToFile(id ?? this.fileId)
                this.openSidebarForFile(id ?? this.fileId)
-               this.handleOpenFile(id ?? null)
+               this.handleOpenFileOnLoad(id ?? null)
        },

        beforeDestroy() {
@@ -242,6 +242,18 @@ export default defineComponent({
                        }
                },

+               handleOpenFileOnLoad(fileId: number|null) {
+                       // This is a workaround as the viewer api registers file actions too late
+                       // Some might not be ready yet
+                       if (document.readyState === 'complete') {
+                               this.$nextTick(() => this.handleOpenFile(fileId))
+                       } else {
+                               window.addEventListener('load', () => {
+                                       this.$nextTick(() => this.handleOpenFile(fileId))
+                               })
+                       }
+               },
+
                /**
                 * Handle opening a file (e.g. by ?openfile=true)
                 * @param fileId File to open
                 * 

@susnux
Copy link

susnux commented Aug 3, 2024

kind of deferring the handleOpenFile until the DOMContentLoaded event from viewer has registered the file actions.

This I would say makes sense, though it would cause visual delays on slow connection. But should be a good fix.

@mmeeks
Copy link
Contributor

mmeeks commented Aug 5, 2024

Thanks Julius - this plagues me many times per day =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Waiting for a developer bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants