Fix zotero://open/ and FileHandlers regressions (#3761)

chrome/content/zotero/xpcom/fileHandlers.js

@@ -81,7 +81,7 @@ Zotero.FileHandlers = {
 			handlers = this._handlersLinux[readerType];
 		}
 		
-		let page = location?.position?.pageIndex ?? undefined;
+		let page = location?.pageIndex ?? undefined;
 		// Add 1 to page index for external readers
 		if (page !== undefined && parseInt(page) == page) {
 			page = parseInt(page) + 1;
@@ -492,9 +492,7 @@ Zotero.OpenPDF = {
 		await Zotero.FileHandlers.open(pathOrItem, {
 			location: {
 				annotationID: annotationKey,
-				position: {
-					pageIndex: page,
-				}
+				pageIndex: page,
 			}
 		});
 	}

components/zotero-protocol-handler.js

@@ -1193,32 +1193,33 @@ function ZoteroProtocolHandler() {
 				return;
 			}
 			
-			var location = null;
+			var location = {};
+			
 			if (page) {
-				location = {
-					position: {
-						pageIndex: page
-					},
-					annotationID: annotation
-				};
+				location.pageIndex = parseInt(page);
 			}
-			else if (cfi) {
-				location = {
-					position: {
-						type: 'FragmentSelector',
-						conformsTo: 'http://www.idpf.org/epub/linking/cfi/epub-cfi.html',
-						value: cfi
-					}
+			if (annotation) {
+				location.annotationID = annotation;
+			}
+			
+			if (cfi) {
+				location.position = {
+					type: 'FragmentSelector',
+					conformsTo: 'http://www.idpf.org/epub/linking/cfi/epub-cfi.html',
+					value: cfi
 				};
 			}
 			else if (sel) {
-				location = {
-					position: {
-						type: 'CssSelector',
-						value: sel
-					}
+				location.position = {
+					type: 'CssSelector',
+					value: sel
 				};
 			}
+			
+			// Don't pass empty location
+			if (!Object.keys(location).length) {
+				location = null;
+			}
 
 			var openInWindow = Zotero.Prefs.get('openReaderInNewWindow');
 

test/tests/fileHandlersTest.js

@@ -29,15 +29,31 @@ describe("Zotero.FileHandlers", () => {
 		it("should open a PDF internally when no handler is set", async function () {
 			let pdf = await importFileAttachment('wonderland_short.pdf');
 			await Zotero.FileHandlers.open(pdf, {
-				location: { position: { pageIndex: 2 } }
+				location: { pageIndex: 2 }
 			});
-			assert.ok(Zotero.Reader.getByTabID(win.Zotero_Tabs.selectedID));
+			let reader = Zotero.Reader.getByTabID(win.Zotero_Tabs.selectedID);
+			assert.ok(reader);
+			
+			// Wait for _setState() (is there an easier way to do this?)
+			let stub = sinon.stub(reader, '_setState');
+			let setStatePromise = new Promise((resolve) => {
+				stub.callsFake(async (...args) => {
+					await stub.wrappedMethod.apply(reader, args);
+					resolve();
+				});
+			});
+			await reader._waitForReader();
+			await setStatePromise;
+			
+			// Check that the reader navigated to the correct page
+			assert.equal(pdf.getAttachmentLastPageIndex(), 2);
+			stub.restore();
 		});
 
 		it("should open a PDF in a new window when no handler is set and openInWindow is passed", async function () {
 			let pdf = await importFileAttachment('wonderland_short.pdf');
 			await Zotero.FileHandlers.open(pdf, {
-				location: { position: { pageIndex: 2 } },
+				location: { pageIndex: 2 },
 				openInWindow: true
 			});
 			assert.notOk(Zotero.Reader.getByTabID(win.Zotero_Tabs.selectedID));