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

Several link handling fixes #5422

Merged
merged 7 commits into from
Mar 1, 2024
Merged
6 changes: 3 additions & 3 deletions src/components/Menu/ActionInsertLink.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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
Expand Down
65 changes: 12 additions & 53 deletions src/extensions/LinkBubblePluginView.js
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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) => {
Expand All @@ -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()
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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

Expand Down
9 changes: 7 additions & 2 deletions src/helpers/links.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
*
*/

import { loadState } from '@nextcloud/initial-state'
import { generateUrl } from '@nextcloud/router'

const domHref = function(node, relativePath) {
Expand All @@ -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
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down
3 changes: 2 additions & 1 deletion src/marks/Link.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
37 changes: 26 additions & 11 deletions src/tests/helpers/links.spec.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { domHref, parseHref } from '../../helpers/links'
import { loadState } from '@nextcloud/initial-state'

global.OCA = {
Viewer: {
Expand All @@ -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', () => {
Expand All @@ -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')
})

})
Expand All @@ -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')
})

})
Expand All @@ -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', () => {
Expand All @@ -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')
})
})
Loading