[stable27] fix(files): race condition on web files view change

[ShGKme]

• Resolves: [Bug]: open shared folder doesn't work #39565

Summary

A small regression from #35772

This issue does not exist on Nextcloud 28 due to full files to view migration.

How to reproduce

1. Share "Folder A" with some "User"
2. Log in as "User"
3. Open Files
4. Open "Shared with you"
5. Click on "Folder A"

• Expected behavior:
  • it opens the sahred folder
  • only "/FolderA" files are requested in "Network"
• Actual behavior:
  • root files are shown for a second or as the result
  • "/" and "/FolderA" twice are requested

Try several times to reproduce. Then more files in root then more chances to reproduce.

bug.mp4

Root of the issue

There is a race condition.

Opening a shared folder makes 2 actions:

server/apps/files_sharing/js/app.js

Lines 260 to 263 in 6bfbb47

	fileActions.register('dir', 'Open', OC.PERMISSION_READ, '', function(filename, context) {
	OCA.Files.App.setActiveView('files', { silent: true })
	OCA.Files.App.fileList.changeDirectory(OC.joinPaths(context.$file.attr('data-path'), filename), true, true)
	})

Changing view from "sharingin" to "files" emits files:legacy-navigation:changed

server/apps/files/js/app.js

Lines 223 to 226 in 6bfbb47

	setActiveView: function(viewId) {
	// The Navigation API will handle the final event
	window._nc_event_bus.emit('files:legacy-navigation:changed', { id: viewId })
	},

Then the Files view is open on root path / which causes fetching the root files list.

server/apps/files/src/views/Navigation.vue

Lines 233 to 242 in 6bfbb47

	onLegacyNavigationChanged({ id } = { id: 'files' }) {
	const view = this.Navigation.views.find(view => view.id === id)
	if (view && view.legacy && view.id !== this.currentView.id) {
	// Force update the current route as the request comes
	// from the legacy files app router
	this.$router.replace({ ...this.$route, params: { view: view.id } })
	this.Navigation.setActive(view)
	this.showView(view)
	}
	},

See this.show:

server/apps/files/src/views/Navigation.vue

Lines 214 to 219 in 2aa15a1

	const { dir = '/' } = OC.Util.History.parseUrlQuery()
	const params = { itemId: view.id, dir }
	logger.debug('Triggering legacy navigation event', params)
	window.jQuery(newAppContent).trigger(new window.jQuery.Event('show', params))
	window.jQuery(newAppContent).trigger(new window.jQuery.Event('urlChanged', params))

And the handler:

server/apps/files/js/filelist.js

Lines 779 to 788 in 2aa15a1

	_onUrlChanged: function(e) {
	if (e && _.isString(e.dir)) {
	var currentDir = this.getCurrentDirectory();
	// this._currentDirectory is NULL when fileList is first initialised
	if(this._currentDirectory && currentDir === e.dir) {
	return;
	}
	this.changeDirectory(e.dir, true, true, undefined, true);
	}
	},

Here is were the first request is made.

Then the initial changeDirectory is called. Which also results in changing directory (to the correct one) and fetching the actual shared directory files list.

🏎️ Here is a race. When the root directory is large, its response may come later so root files are shown instead of the shared folder.

---

Changing view also triggers a shown event that sets the current directory to / and reloads content. It causes duplicated file requests.

server/apps/files/js/filelist.js

Lines 765 to 774 in 2aa15a1

	_onShow: function(e) {
	OCA.Files.App && OCA.Files.App.updateCurrentFileList(this);
	if (e.itemId === this.id) {
	this._setCurrentDir('/', false);
	}
	// Only reload if we don't navigate to a different directory
	if (typeof e.dir === 'undefined' || e.dir === this.getCurrentDirectory()) {
	this.reload();
	}
	},

Before Vue migration, there was a check for this.shown that avoided this behavior.

server/apps/files/js/filelist.js

Lines 761 to 773 in f720d6e

	_onShow: function(e) {
	OCA.Files.App && OCA.Files.App.updateCurrentFileList(this);
	if (this.shown) {
	if (e.itemId === this.id) {
	this._setCurrentDir('/', false);
	}
	// Only reload if we don't navigate to a different directory
	if (typeof e.dir === 'undefined' || e.dir === this.getCurrentDirectory()) {
	this.reload();
	}
	}
	this.shown = true;
	},

If I'm not wrong, with migrating navigation to Vue, we can just remove this part. cc @skjnldsv

TODO

• Add silent mode support for legacy navigation changed. Actually, the same that was there before migration to Vue.
• Remove resetting and reloading on view show

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)

[ShGKme]

/backport to stable26

[skjnldsv]

Make sense! Nice catch, this legacy router is a pain 🥖

[ShGKme]

Want to check one more case with page reload when switching from the shared folder to "All files".

[ShGKme]

@skjnldsv Do know any edge cases with legacy navigation/apps here that should be tested?

[ShGKme]

Want to check one more case with page reload when switching from the shared folder to "All files".

Unrelated to this issue.

OC.Util.History stores query param as a string in a native history.state. vue-router stores its own state there and crashes found a string there.

[ShGKme]

/backport to stable26