Merge pull request #5422 from nextcloud/fix/link_handling2

Several link handling fixes

Author: juliusknorr

src/components/Menu/ActionInsertLink.vue

@@ -50,7 +50,7 @@
 		</NcActionButton>
 		<NcActionInput v-if="isInputMode"
 			type="text"
-			:value="href"
+			:value.sync="href"
 			:data-text-action-entry="`${actionEntry.key}-input`"
 			@submit="linkWebsite">
 			<template #icon>
@@ -144,8 +144,8 @@ export default {
 				.then((file) => {
 					const client = OC.Files.getClient()
 					client.getFileInfo(file).then((_status, fileInfo) => {
-						const href = generateUrl(`/f/${fileInfo.id}`)
-						this.setLink(href, fileInfo.name)
+						const url = new URL(generateUrl(`/f/${fileInfo.id}`), window.origin)
+						this.setLink(url.href, fileInfo.name)
 						this.startPath = fileInfo.path + (fileInfo.type === 'dir' ? `/${fileInfo.name}/` : '')
 					})
 					this.menuOpen = false

src/extensions/LinkBubblePluginView.js

@@ -1,18 +1,15 @@
 import { VueRenderer } from '@tiptap/vue-2'
 import tippy from 'tippy.js'
+import debounce from 'debounce'
 import { domHref } from '../helpers/links.js'
 import LinkBubbleView from '../components/Link/LinkBubbleView.vue'
 
 import { getViewerVue } from '../ViewerVue.js'
 
-const updateDelay = 250
-
 class LinkBubblePluginView {
 
 	#component = null
-	#preventHide = false
 	#hadUpdateFromClick = false
-	#updateDebounceTimer = undefined
 
 	constructor({ editor, view }) {
 		this.editor = editor
@@ -33,18 +30,16 @@ class LinkBubblePluginView {
 		this.view.dom.addEventListener('dragstart', this.dragOrScrollHandler)
 		this.view.dom.addEventListener('click', this.clickHandler)
 		document.addEventListener('scroll', this.dragOrScrollHandler, { capture: true })
-		this.editor.on('focus', this.focusHandler)
-		this.editor.on('blur', this.blurHandler)
 	}
 
-	dragOrScrollHandler = () => {
+	dragOrScrollHandler = (event) => {
+		// Cypress fires unexpected scroll events, which breaks testing the link bubble
+		if (window.Cypress) {
+			return
+		}
 		this.hide()
 	}
 
-	pointerdownHandler = () => {
-		this.#preventHide = true
-	}
-
 	// Required for read-only mode on Firefox. For some reason, editor selection doesn't get
 	// updated when clicking a link in read-only mode on Firefox.
 	clickHandler = (event) => {
@@ -67,28 +62,6 @@ class LinkBubblePluginView {
 		setTimeout(() => this.updateFromClick(this.editor.view, clickedPos))
 	}
 
-	focusHandler = () => {
-		// we use `setTimeout` to make sure `selection` is already updated
-		setTimeout(() => this.update(this.editor.view))
-	}
-
-	blurHandler = ({ event }) => {
-		if (this.#preventHide) {
-			this.#preventHide = false
-			return
-		}
-
-		if (event?.relatedTarget && this.tippy?.popper.firstChild.contains(event.relatedTarget)) {
-			return
-		}
-
-		this.hide()
-	}
-
-	tippyBlurHandler = () => {
-		this.hide()
-	}
-
 	keydownHandler = (event) => {
 		if (event.key === 'Escape') {
 			event.preventDefault()
@@ -116,10 +89,6 @@ class LinkBubblePluginView {
 				strategy: 'fixed',
 			},
 		})
-
-		this.tippy.popper.firstChild?.addEventListener('pointerdown', this.pointerdownHandler, { capture: true })
-		// Hide tippy on its own blur event as well
-		this.tippy.popper.firstChild?.addEventListener('blur', this.tippyBlurHandler)
 	}
 
 	update(view, oldState) {
@@ -132,16 +101,10 @@ class LinkBubblePluginView {
 			return
 		}
 
-		if (this.#updateDebounceTimer) {
-			clearTimeout(this.#updateDebounceTimer)
-		}
-
-		this.#updateDebounceTimer = window.setTimeout(() => {
-			this.updateFromSelection(view)
-		}, updateDelay)
+		this.updateFromSelection(view)
 	}
 
-	updateFromSelection(view) {
+	updateFromSelection = debounce((view) => {
 		// Don't update directly after updateFromClick. Prevents race condition in read-only documents in Chrome.
 		if (this.#hadUpdateFromClick) {
 			return
@@ -164,7 +127,7 @@ class LinkBubblePluginView {
 		const shouldShow = !!linkNode && hasEditorFocus
 
 		this.updateTooltip(view, shouldShow, linkNode, nodeStart)
-	}
+	}, 250)
 
 	updateFromClick(view, clickedLinkPos) {
 		const nodeStart = clickedLinkPos.pos - clickedLinkPos.textOffset
@@ -174,11 +137,11 @@ class LinkBubblePluginView {
 		this.#hadUpdateFromClick = true
 		setTimeout(() => {
 			this.#hadUpdateFromClick = false
-		}, 200)
+		}, 500)
 		this.updateTooltip(this.editor.view, shouldShow, clickedNode, nodeStart)
 	}
 
-	updateTooltip = (view, shouldShow, linkNode, nodeStart) => {
+	updateTooltip(view, shouldShow, linkNode, nodeStart) {
 		this.createTooltip()
 
 		if (!shouldShow || !linkNode) {
@@ -216,17 +179,13 @@ class LinkBubblePluginView {
 	}
 
 	destroy() {
-		this.tippy?.popper.firstChild?.removeEventListener('blur', this.tippyBlurHandler)
-		this.tippy?.popper.firstChild?.removeEventListener('pointerdown', this.pointerdownHandler, { capture: true })
 		this.tippy?.destroy()
 		this.view.dom.removeEventListener('dragstart', this.dragOrScrollHandler)
 		this.view.dom.removeEventListener('click', this.clickHandler)
 		document.removeEventListener('scroll', this.dragOrScrollHandler, { capture: true })
-		this.editor.off('focus', this.focusHandler)
-		this.editor.off('blur', this.blurHandler)
 	}
 
-	linkNodeFromSelection = (view) => {
+	linkNodeFromSelection(view) {
 		const { state } = view
 		const { selection } = state
 

src/helpers/links.js

@@ -20,6 +20,7 @@
  *
  */
 
+import { loadState } from '@nextcloud/initial-state'
 import { generateUrl } from '@nextcloud/router'
 
 const domHref = function(node, relativePath) {
@@ -36,6 +37,10 @@ const domHref = function(node, relativePath) {
 	if (ref.startsWith('#')) {
 		return ref
 	}
+	// Don't rewrite links in collectives app context
+	if (loadState('core', 'active-app') === 'collectives') {
+		return ref
+	}
 	// Don't rewrite links to the collectives app
 	if (ref.includes('/apps/collectives/')) {
 		return ref
@@ -45,7 +50,7 @@ const domHref = function(node, relativePath) {
 	const match = ref.match(/^([^?]*)\?fileId=(\d+)/)
 	if (match) {
 		const [, , id] = match
-		return generateUrl(`/f/${id}`)
+		return (new URL(generateUrl(`/f/${id}`), window.origin)).href
 	}
 	return ref
 }
@@ -58,7 +63,7 @@ const parseHref = function(dom) {
 	const match = ref.match(/\?dir=([^&]*)&openfile=([^&]*)#relPath=([^&]*)/)
 	if (match) {
 		const [, , id] = match
-		return generateUrl(`/f/${id}`)
+		return (new URL(generateUrl(`/f/${id}`), window.origin)).href
 	}
 	return ref
 }

src/marks/Link.js

@@ -82,9 +82,10 @@ const Link = TipTapLink.extend({
 				props: {
 					handleDOMEvents: {
 						// Open link in new tab on middle click
-						pointerup: (view, event) => {
+						auxclick: (view, event) => {
 							if (event.target.closest('a') && event.button === 1 && !event.ctrlKey && !event.metaKey && !event.shiftKey) {
 								event.preventDefault()
+								event.stopImmediatePropagation()
 
 								const linkElement = event.target.closest('a')
 								window.open(linkElement.href, '_blank')

src/tests/helpers/links.spec.js

@@ -1,4 +1,5 @@
 import { domHref, parseHref } from '../../helpers/links'
+import { loadState } from '@nextcloud/initial-state'
 
 global.OCA = {
 	Viewer: {
@@ -12,6 +13,9 @@ global.OC = {
 
 global._oc_webroot = ''
 
+jest.mock('@nextcloud/initial-state')
+loadState.mockImplementation((app, key) => 'files')
+
 describe('Preparing href attributes for the DOM', () => {
 
 	test('leave empty hrefs alone', () => {
@@ -34,22 +38,22 @@ describe('Preparing href attributes for the DOM', () => {
 
 	test('relative link with fileid (old format from file picker)', () => {
 		expect(domHref({attrs: {href: 'otherfile?fileId=123'}}))
-			.toBe('/f/123')
+			.toBe('http://localhost/f/123')
 	})
 
 	test('relative path with ../ (old format from file picker)', () => {
 		expect(domHref({attrs: {href: '../other/otherfile?fileId=123'}}))
-			.toBe('/f/123')
+			.toBe('http://localhost/f/123')
 	})
 
 	test('absolute path (old format from file picker)', () => {
 		expect(domHref({attrs: {href: '/other/otherfile?fileId=123'}}))
-			.toBe('/f/123')
+			.toBe('http://localhost/f/123')
 	})
 
 	test('absolute path (old format from file picker)', () => {
 		expect(domHref({attrs: {href: '/otherfile?fileId=123'}}))
-			.toBe('/f/123')
+			.toBe('http://localhost/f/123')
 	})
 
 })
@@ -72,7 +76,7 @@ describe('Extracting short urls from the DOM', () => {
 
 	test('relative link with fileid (old format from file picker)', () => {
 		expect(parseHref(domStub('?dir=/other&openfile=123#relPath=../other/otherfile')))
-			.toBe('/f/123')
+			.toBe('http://localhost/f/123')
 	})
 
 })
@@ -99,22 +103,22 @@ describe('Inserting hrefs into the dom and extracting them again', () => {
 
 	test('old relative link format (from file picker) is rewritten', () => {
 		expect(insertAndExtract({href: 'otherfile?fileId=123'}))
-			.toBe('/f/123')
+			.toBe('http://localhost/f/123')
 	})
 
 	test('old relative link format with ../ (from file picker) is rewritten', () => {
 		expect(insertAndExtract({href: '../otherfile?fileId=123'}))
-			.toBe('/f/123')
+			.toBe('http://localhost/f/123')
 	})
 
 	test('old absolute link format (from file picker) is rewritten', () => {
 		expect(insertAndExtract({href: '/otherfile?fileId=123'}))
-			.toBe('/f/123')
+			.toBe('http://localhost/f/123')
 	})
 
-	test('default absolute link format is unchanged', () => {
-		expect(insertAndExtract({href: '/f/123'}))
-			.toBe('/f/123')
+	test('default full URL link format is unchanged', () => {
+		expect(insertAndExtract({href: 'http://localhost/f/123'}))
+			.toBe('http://localhost/f/123')
 	})
 
 	test('absolute link to collectives page is unchanged', () => {
@@ -123,3 +127,14 @@ describe('Inserting hrefs into the dom and extracting them again', () => {
 	})
 
 })
+
+describe('Preparing href attributes for the DOM in Collectives app', () => {
+	beforeAll(() => {
+		loadState.mockImplementation((app, key) => 'collectives')
+	})
+
+	test('relative link with fileid in Collectives', () => {
+		expect(domHref({attrs: {href: 'otherfile?fileId=123'}}))
+			.toBe('otherfile?fileId=123')
+	})
+})