Skip to content

fix(mobile): correctly load via direct editing#5805

Merged
elzody merged 1 commit into
mainfrom
fix/mobile-editing
Jul 9, 2026
Merged

fix(mobile): correctly load via direct editing#5805
elzody merged 1 commit into
mainfrom
fix/mobile-editing

Conversation

@elzody

@elzody elzody commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

The mobile apps should now receive the loaded() event and OCSController::createDirect() should now always return properly for legacy (<34) clients and prevent a 404 error.

Assisted-by: ClaudeCode:claude-sonnet-4-6

TODO

  • Tests

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Documentation (manuals or wiki) has been updated or is not required

@elzody elzody self-assigned this Jun 29, 2026
@elzody elzody added the 2. developing Work in progress label Jun 29, 2026
@elzody elzody force-pushed the fix/mobile-editing branch 2 times, most recently from 6539065 to 3bb44ec Compare July 8, 2026 20:42
@elzody elzody changed the title fix(mobile): support server direct editing API for Android v34+ and f… fix(mobile): correctly load via direct editing Jul 8, 2026
@elzody elzody force-pushed the fix/mobile-editing branch from 6edf79a to 4900e2f Compare July 8, 2026 21:21
Assisted-by: ClaudeCode:claude-sonnet-4-6
Signed-off-by: Elizabeth Danzberger <elizabeth@elzody.dev>
* legacy /apps/richdocuments/api/v1/document endpoint and should not
* see the editor in OCP\DirectEditing discovery responses.
*/
private const MIN_MOBILE_CLIENT_VERSION = 34;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why is this comment deleted? Is this not useful to know?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

IMO it is self-explanatory from the file itself. You can tell quite clearly that it is used only to expose the editor for mobile clients >=34

@moodyjmz

moodyjmz commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

So at the risk of being really verbose:

The createDirect() change is right: returning the native richdocuments.directView.show webview instead of files.DirectEditingView.edit means the URL no longer depends on the editor being registered through OCP\DirectEditing, so it renders regardless of client version. The DirectEditingMobileInterface forwarding in mobile.js matches what text already does for the shared direct-editing webview, and token handling is unchanged (64-char CSPRNG, single-use, 10-min TTL), no new exposure.

What still fails: createFromTemplate() (POST /api/v1/templates/new) wasn't given the same treatment, it still returns a files.DirectEditingView.edit URL. So a <v34 mobile client creating a new document from a template:

  1. POSTs to /api/v1/templates/new; the endpoint force-registers the editor inline, create() succeeds, and hands back a files.DirectEditingView.edit?token=… URL.
  2. Loads that URL in its webview. The GET carries the legacy iOS/Android user-agent, so RegisterDirectEditorListener::shouldExposeEditor() gates richdocuments out (major < 34), the token's editor can't be resolved, and the render fails — 404 on iOS, infinite spinner on Android.

Same symptom this PR fixes for opening, on the same entry-point family. Reachable from the shipping apps — Android hits /api/v1/templates/new via CreateFileFromTemplateOperation, iOS via NextcloudKit/ios-communication-library — so "new doc from template" on a v33 app against a v34 server still breaks after this merges.

createFromTemplate is the only endpoint still on the gated flow: createDirect, createPublic, createPublicFromInitiator already use richdocuments.directView.show.

Fix (same shape as the createDirect change here): recreate the file explicitly and carry the template id on the direct token; DirectViewController::show() already primes the template via primeTemplateSource(). As a bonus this removes the last users of directEditingManager/officeDirectEditor, so those imports and constructor deps can go too. You could fold it into here, or file a new issue and create a new PR for better tracking.

I've not checked this code, so verify it please:

1. lib/Controller/OCSController.phpcreateFromTemplate() (replaces lines 290–336)

	public function createFromTemplate($path, $template) {
		if ($path === null || $template === null) {
			throw new OCSBadRequestException('path and template must be set');
		}

		if (!$this->manager->isTemplate($template)) {
			throw new OCSBadRequestException('Invalid template provided');
		}

		try {
			$info = $this->mb_pathinfo($path);

			$userFolder = $this->rootFolder->getUserFolder($this->userId);
			$folder = isset($info['dirname']) ? $userFolder->get($info['dirname']) : $userFolder;
			$name = $folder->getNonExistingName($info['basename']);
			$file = $folder->newFile($name);

			$direct = $this->directMapper->newDirect($this->userId, $file->getId(), $template);

			return new DataResponse([
				'url' => $this->urlGenerator->linkToRouteAbsolute('richdocuments.directView.show', [
					'token' => $direct->getToken(),
				]),
			]);
		} catch (NotFoundException) {
			throw new OCSNotFoundException();
		} catch (\Exception $e) {
			$this->logger->error($e->getMessage(), ['exception' => $e]);
			throw new OCSException('Failed to create new file from template.');
		}
	}

Notes:

  • newFile($name) recreates the empty file (create() used to do this); newDirect(..., $template) carries the template id on the token.
  • Dropped the catch (OCSBadRequestException $e) { throw $e; } — nothing inside the try throws it any more (the getTemplateTypeForMime null-check went away; isTemplate() validation stays at the top).
  • Mime-based creator derivation is no longer needed — it only mattered for create()'s creator routing; the native flow resolves the type from the file/template on render.

2. lib/Controller/OCSController.php — now-dead imports / deps to remove

// remove these imports:
use OCA\Richdocuments\AppInfo\Application;                 // line 12
use OCA\Richdocuments\DirectEditing\OfficeDirectEditor;    // line 14
use OCP\DirectEditing\IManager as IDirectEditingManager;   // line 27

// remove these constructor params:
private IDirectEditingManager $directEditingManager,       // line 54
private OfficeDirectEditor $officeDirectEditor,            // line 55

(These were the only remaining users after createDirect was reverted; safe to drop — nothing external constructs this controller.)

3. cypress/e2e/direct.spec.js — new test mirroring the open-path one

	it('Signals DirectEditingMobileInterface when creating from a template', function() {
		getTemplates(randUser, 'document')
			.then((templates) => {
				const emptyTemplate = templates.find((template) => template.name === 'Empty')
				createNewFileDirectEditingLink(randUser, 'mobile-template.odt', emptyTemplate.id)
					.then((token) => {
						cy.logout()
						cy.visit(token, {
							onBeforeLoad(win) {
								win.DirectEditingMobileInterface = {
									loaded: cy.stub().as('directEditingLoaded'),
									documentLoaded: cy.stub().as('directEditingDocumentLoaded'),
									close: cy.stub().callsFake(() => win.documentsMain.onClose()),
								}
								cy.spy(win, 'postMessage').as('postMessage')
							},
						})
						cy.waitForCollabora(false)
						cy.waitForPostMessage('App_LoadingStatus', { Status: 'Document_Loaded' })
						cy.get('@directEditingLoaded').should('have.been.called')
						cy.get('@directEditingDocumentLoaded').should('have.been.called')
						cy.closeDirectDocument()
					})
			})
	})

Also add mobile-template.odt to the spec's afterEach cleanup (currently deletes document.odt/mynewfile.odt/document.rtf) so repeated local runs don't accumulate mobile-template (2).odt via getNonExistingName.

@moodyjmz moodyjmz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

However, approval does mean the other issue gets filed and fixed too

@elzody

elzody commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Yes, we can file this as a separate issue and get that fixed as well.

@elzody

elzody commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

/backport to stable34 please

@elzody elzody merged commit 8dc7187 into main Jul 9, 2026
78 of 80 checks passed
@elzody elzody deleted the fix/mobile-editing branch July 9, 2026 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Ready to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Nextcloud Office with my Collabora server no longer works with mobile devices from version 34.0

2 participants