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
juliusknorr opened this issue Jul 9, 2024 · 11 comments
Open

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

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

Comments

@juliusknorr
Copy link
Member

juliusknorr 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
@juliusknorr juliusknorr added bug Something isn't working 1. to develop Waiting for a developer labels Jul 9, 2024
@juliusknorr
Copy link
Member Author

juliusknorr 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.

@juliusknorr
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.

@juliusknorr
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 =)

@pedropintosilva
Copy link
Contributor

pedropintosilva commented Oct 15, 2024

Hello and thanks for the nextcloud/viewer#2486 @susnux! I see that @artonge has tried to backported with nextcloud/viewer#2490 to [stable29] but it seems there is the need to rebase it and there are some conflicts. If there is anything I can test please let me know.

@pedropintosilva
Copy link
Contributor

Thanks @artonge , I see it's not in! Good so it should be include in 28 and 29 release. The fix is include 3.0.1 (17 oct).

@AnnaNazaryan : (to be confirmed) it seems in our staging it's still reproducible (30.0.2 RC1) cc: @jazevedo-coll

@jazevedo-coll
Copy link

(to be confirmed) it seems in our staging it's still reproducible (30.0.2 RC1)

@pedropintosilva what I have seen with our staging instance is: when opening an internal link on a browser with a clean cache and history and site settings, sometimes the file will not open and will just download instead.

Image

@mmeeks
Copy link
Contributor

mmeeks commented Nov 7, 2024

Is it possible that there is some async / defer / fetchpriority foo going on that sometimes perturbs the order of execution and initialization of the different JS modules themselves ?

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

6 participants