diff options
| -rw-r--r-- | apps/files/src/components/FileEntry/FileEntryName.vue | 46 | ||||
| -rw-r--r-- | apps/files/src/components/NewNodeDialog.vue | 309 | ||||
| -rw-r--r-- | apps/files/src/utils/filenameValidity.ts | 41 | ||||
| -rw-r--r-- | cypress/e2e/files/FilesUtils.ts | 9 | ||||
| -rw-r--r-- | cypress/e2e/files/files-renaming.cy.ts | 77 | ||||
| -rw-r--r-- | package-lock.json | 2 | ||||
| -rw-r--r-- | package.json | 2 |
7 files changed, 273 insertions, 213 deletions
diff --git a/apps/files/src/components/FileEntry/FileEntryName.vue b/apps/files/src/components/FileEntry/FileEntryName.vue index c73972f3ead..307a9c78330 100644 --- a/apps/files/src/components/FileEntry/FileEntryName.vue +++ b/apps/files/src/components/FileEntry/FileEntryName.vue @@ -44,7 +44,7 @@ class="files-list__row-name-link" data-cy-files-list-row-name-link v-bind="linkTo.params"> - <!-- File name --> + <!-- Filename --> <span class="files-list__row-name-text"> <!-- Keep the filename stuck to the extension to avoid whitespace rendering issues--> <span class="files-list__row-name-" v-text="basename" /> @@ -60,7 +60,7 @@ import type { PropType } from 'vue' import axios from '@nextcloud/axios' import { showError, showSuccess } from '@nextcloud/dialogs' import { emit } from '@nextcloud/event-bus' -import { FileType, NodeStatus } from '@nextcloud/files' +import { FileType, InvalidFilenameError, InvalidFilenameErrorReason, NodeStatus, validateFilename } from '@nextcloud/files' import { loadState } from '@nextcloud/initial-state' import { translate as t } from '@nextcloud/l10n' import { isAxiosError} from 'axios' @@ -149,7 +149,7 @@ export default Vue.extend({ renameLabel() { const matchLabel: Record<FileType, string> = { - [FileType.File]: t('files', 'File name'), + [FileType.File]: t('files', 'Filename'), [FileType.Folder]: t('files', 'Folder name'), } return matchLabel[this.source.type] @@ -187,7 +187,7 @@ export default Vue.extend({ watch: { /** - * If renaming starts, select the file name + * If renaming starts, select the filename * in the input, without the extension. * @param renaming */ @@ -222,27 +222,35 @@ export default Vue.extend({ input.reportValidity() } }, - isFileNameValid(name) { - const trimmedName = name.trim() - const char = trimmedName.indexOf('/') !== -1 - ? '/' - : forbiddenCharacters.find((char) => trimmedName.includes(char)) - - if (trimmedName === '.' || trimmedName === '..') { - throw new Error(t('files', '"{name}" is an invalid file name.', { name })) - } else if (trimmedName.length === 0) { + isFileNameValid(name: string) { + if (name.trim() === '') { throw new Error(t('files', 'File name cannot be empty.')) - } else if (char) { - throw new Error(t('files', '"{char}" is not allowed inside a file name.', { char })) - } else if (trimmedName.match(OC.config.blacklist_files_regex)) { - throw new Error(t('files', '"{name}" is not an allowed filetype.', { name })) } else if (this.checkIfNodeExists(name)) { throw new Error(t('files', '{newName} already exists.', { newName: name })) } - return true + try { + validateFilename(name) + } catch (error) { + if (!(error instanceof InvalidFilenameError)) { + logger.error(error as Error) + return + } + switch (error.reason) { + case InvalidFilenameErrorReason.Character: + throw new Error(t('files', '"{segment}" is not allowed inside a filename.', { segment: error.segment })) + case InvalidFilenameErrorReason.ReservedName: + throw new Error(t('files', '"{segment}" is a forbidden file or folder name.', { segment: error.segment })) + case InvalidFilenameErrorReason.Extension: + if (error.segment.startsWith('.')) { + throw new Error(t('files', '"{segment}" is not an allowed filetype.', { segment: error.segment })) + } else { + throw new Error(t('files', 'Filenames must not end with "{segment}".', { segment: error.segment })) + } + } + } }, - checkIfNodeExists(name) { + checkIfNodeExists(name: string) { return this.nodes.find(node => node.basename === name && node !== this.source) }, diff --git a/apps/files/src/components/NewNodeDialog.vue b/apps/files/src/components/NewNodeDialog.vue index 0f6a739d21c..b4647724de3 100644 --- a/apps/files/src/components/NewNodeDialog.vue +++ b/apps/files/src/components/NewNodeDialog.vue @@ -1,226 +1,157 @@ <!-- - - @copyright Copyright (c) 2024 Ferdinand Thiessen <opensource@fthiessen.de> - - - - @author Ferdinand Thiessen <opensource@fthiessen.de> - - - - @license AGPL-3.0-or-later - - - - This program is free software: you can redistribute it and/or modify - - it under the terms of the GNU Affero General Public License as - - published by the Free Software Foundation, either version 3 of the - - License, or (at your option) any later version. - - - - This program is distributed in the hope that it will be useful, - - but WITHOUT ANY WARRANTY; without even the implied warranty of - - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - - GNU Affero General Public License for more details. - - - - You should have received a copy of the GNU Affero General Public License - - along with this program. If not, see <http://www.gnu.org/licenses/>. - - - --> + - SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors + - SPDX-License-Identifier: AGPL-3.0-or-later +--> <template> - <NcDialog :name="name" + <NcDialog data-cy-files-new-node-dialog + :name="name" :open="open" close-on-click-outside out-transition - @update:open="onClose"> + @update:open="emit('close', null)"> <template #actions> - <NcButton type="primary" - :disabled="!isUniqueName" - @click="onCreate"> + <NcButton data-cy-files-new-node-dialog-submit + type="primary" + :disabled="validity !== ''" + @click="submit"> {{ t('files', 'Create') }} </NcButton> </template> - <form @submit.prevent="onCreate"> - <NcTextField ref="input" - class="dialog__input" - :error="!isUniqueName" - :helper-text="errorMessage" + <form ref="formElement" + class="new-node-dialog__form" + @submit.prevent="emit('close', localDefaultName)"> + <NcTextField ref="nameInput" + data-cy-files-new-node-dialog-input + :error="validity !== ''" + :helper-text="validity" :label="label" - :value.sync="localDefaultName" - @keyup="checkInputValidity" /> + :value.sync="localDefaultName" /> </form> </NcDialog> </template> -<script lang="ts"> -import type { PropType } from 'vue' - -import { defineComponent } from 'vue' -import { translate as t } from '@nextcloud/l10n' +<script setup lang="ts"> +import type { ComponentPublicInstance, PropType } from 'vue' import { getUniqueName } from '@nextcloud/files' -import { loadState } from '@nextcloud/initial-state' +import { t } from '@nextcloud/l10n' +import { extname } from 'path' +import { nextTick, onMounted, ref, watch, watchEffect } from 'vue' +import { getFilenameValidity } from '../utils/filenameValidity.ts' import NcButton from '@nextcloud/vue/dist/Components/NcButton.js' import NcDialog from '@nextcloud/vue/dist/Components/NcDialog.js' import NcTextField from '@nextcloud/vue/dist/Components/NcTextField.js' -import logger from '../logger.js' - -interface ICanFocus { - focus: () => void -} -const forbiddenCharacters = loadState<string>('files', 'forbiddenCharacters', '').split('') - -export default defineComponent({ - name: 'NewNodeDialog', - components: { - NcButton, - NcDialog, - NcTextField, +const props = defineProps({ + /** + * The name to be used by default + */ + defaultName: { + type: String, + default: t('files', 'New folder'), }, - props: { - /** - * The name to be used by default - */ - defaultName: { - type: String, - default: t('files', 'New folder'), - }, - /** - * Other files that are in the current directory - */ - otherNames: { - type: Array as PropType<string[]>, - default: () => [], - }, - /** - * Open state of the dialog - */ - open: { - type: Boolean, - default: true, - }, - /** - * Dialog name - */ - name: { - type: String, - default: t('files', 'Create new folder'), - }, - /** - * Input label - */ - label: { - type: String, - default: t('files', 'Folder name'), - }, + /** + * Other files that are in the current directory + */ + otherNames: { + type: Array as PropType<string[]>, + default: () => [], }, - emits: { - close: (name: string|null) => name === null || name, + /** + * Open state of the dialog + */ + open: { + type: Boolean, + default: true, }, - data() { - return { - localDefaultName: this.defaultName || t('files', 'New folder'), - } + /** + * Dialog name + */ + name: { + type: String, + default: t('files', 'Create new folder'), }, - computed: { - errorMessage() { - if (this.isUniqueName) { - return '' - } else { - return t('files', 'A file or folder with that name already exists.') - } - }, - uniqueName() { - return getUniqueName(this.localDefaultName, this.otherNames) - }, - isUniqueName() { - return this.localDefaultName === this.uniqueName - }, + /** + * Input label + */ + label: { + type: String, + default: t('files', 'Folder name'), }, - watch: { - defaultName() { - this.localDefaultName = this.defaultName || t('files', 'New folder') - }, +}) - /** - * Ensure the input is focussed even if the dialog is already mounted but not open - */ - open() { - this.$nextTick(() => this.focusInput()) - }, - }, - mounted() { - // on mounted lets use the unique name - this.localDefaultName = this.uniqueName - this.$nextTick(() => this.focusInput()) - }, - methods: { - t, +const emit = defineEmits<{ + (event: 'close', name: string | null): void +}>() + +const localDefaultName = ref<string>(props.defaultName) +const nameInput = ref<ComponentPublicInstance>() +const formElement = ref<HTMLFormElement>() +const validity = ref('') + +/** + * Focus the filename input field + */ +function focusInput() { + nextTick(() => { + // get the input element + const input = nameInput.value?.$el.querySelector('input') + if (!props.open || !input) { + return + } - /** - * Focus the filename input field - */ - focusInput() { - if (this.open) { - this.$nextTick(() => (this.$refs.input as unknown as ICanFocus)?.focus?.()) - } - }, + // length of the basename + const length = localDefaultName.value.length - extname(localDefaultName.value).length + // focus the input + input.focus() + // and set the selection to the basename (name without extension) + input.setSelectionRange(0, length) + }) +} - onCreate() { - this.$emit('close', this.localDefaultName) - }, - onClose(state: boolean) { - if (!state) { - this.$emit('close', null) - } - }, +/** + * Trigger submit on the form + */ +function submit() { + formElement.value?.requestSubmit() +} - /** - * Check if the file name is valid and update the - * input validity using browser's native validation. - * @param event the keyup event - */ - checkInputValidity(event: KeyboardEvent) { - const input = event.target as HTMLInputElement - const newName = this.localDefaultName.trim?.() || '' - logger.debug('Checking input validity', { newName }) - try { - this.isFileNameValid(newName) - input.setCustomValidity('') - input.title = '' - } catch (e) { - if (e instanceof Error) { - input.setCustomValidity(e.message) - input.title = e.message - } else { - input.setCustomValidity(t('files', 'Invalid file name')) - } - } finally { - input.reportValidity() - } - }, +// Reset local name on props change +watch(() => props.defaultName, () => { + localDefaultName.value = getUniqueName(props.defaultName, props.otherNames) +}) - isFileNameValid(name: string) { - const trimmedName = name.trim() - const char = trimmedName.indexOf('/') !== -1 - ? '/' - : forbiddenCharacters.find((char) => trimmedName.includes(char)) +// Validate the local name +watchEffect(() => { + if (props.otherNames.includes(localDefaultName.value)) { + validity.value = t('files', 'This name is already in use.') + } else { + validity.value = getFilenameValidity(localDefaultName.value) + } + const input = nameInput.value?.$el.querySelector('input') + if (input) { + input.setCustomValidity(validity.value) + input.reportValidity() + } +}) - if (trimmedName === '.' || trimmedName === '..') { - throw new Error(t('files', '"{name}" is an invalid file name.', { name })) - } else if (trimmedName.length === 0) { - throw new Error(t('files', 'File name cannot be empty.')) - } else if (char) { - throw new Error(t('files', '"{char}" is not allowed inside a file name.', { char })) - } else if (trimmedName.match(window.OC.config.blacklist_files_regex)) { - throw new Error(t('files', '"{name}" is not an allowed filetype.', { name })) - } +// Ensure the input is focussed even if the dialog is already mounted but not open +watch(() => props.open, () => { + nextTick(() => { + focusInput() + }) +}) - return true - }, - }, +onMounted(() => { + // on mounted lets use the unique name + localDefaultName.value = getUniqueName(localDefaultName.value, props.otherNames) + nextTick(() => focusInput()) }) </script> -<style lang="scss" scoped> -.dialog__input { - :deep(input:invalid) { - // Show red border on invalid input - border-color: var(--color-error); - color: red; - } +<style scoped> +.new-node-dialog__form { + /* Ensure the dialog does not jump when there is a validity error */ + min-height: calc(3 * var(--default-clickable-area)); } </style> diff --git a/apps/files/src/utils/filenameValidity.ts b/apps/files/src/utils/filenameValidity.ts new file mode 100644 index 00000000000..2666d530052 --- /dev/null +++ b/apps/files/src/utils/filenameValidity.ts @@ -0,0 +1,41 @@ +/*! + * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +import { InvalidFilenameError, InvalidFilenameErrorReason, validateFilename } from '@nextcloud/files' +import { t } from '@nextcloud/l10n' + +/** + * Get the validity of a filename (empty if valid). + * This can be used for `setCustomValidity` on input elements + * @param name The filename + * @param escape Escape the matched string in the error (only set when used in HTML) + */ +export function getFilenameValidity(name: string, escape = false): string { + if (name.trim() === '') { + return t('files', 'Filename must not be empty.') + } + + try { + validateFilename(name) + return '' + } catch (error) { + if (!(error instanceof InvalidFilenameError)) { + throw error + } + + switch (error.reason) { + case InvalidFilenameErrorReason.Character: + return t('files', '"{char}" is not allowed inside a filename.', { char: error.segment }, undefined, { escape }) + case InvalidFilenameErrorReason.ReservedName: + return t('files', '"{segment}" is a reserved name and not allowed for filenames.', { segment: error.segment }, undefined, { escape: false }) + case InvalidFilenameErrorReason.Extension: + if (error.segment.match(/\.[a-z]/i)) { + return t('files', '"{extension}" is not an allowed filetype.', { extension: error.segment }, undefined, { escape: false }) + } + return t('files', 'Filenames must not end with "{extension}".', { extension: error.segment }, undefined, { escape: false }) + default: + return t('files', 'Invalid filename.') + } + } +} diff --git a/cypress/e2e/files/FilesUtils.ts b/cypress/e2e/files/FilesUtils.ts index 3ff459d4c43..98dae12ce97 100644 --- a/cypress/e2e/files/FilesUtils.ts +++ b/cypress/e2e/files/FilesUtils.ts @@ -28,9 +28,12 @@ export const getActionButtonForFile = (filename: string) => getActionsForFile(fi export const triggerActionForFile = (filename: string, actionId: string) => { getActionButtonForFile(filename).click({ force: true }) - cy.get(`[data-cy-files-list-row-action="${CSS.escape(actionId)}"] > button`).should('exist') - cy.get(`[data-cy-files-list-row-action="${CSS.escape(actionId)}"] > button`).scrollIntoView() - cy.get(`[data-cy-files-list-row-action="${CSS.escape(actionId)}"] > button`).click({ force: true }) + cy.get(`[data-cy-files-list-row-action="${CSS.escape(actionId)}"] > button`) + .should('exist') + .scrollIntoView() + cy.get(`[data-cy-files-list-row-action="${CSS.escape(actionId)}"] > button`) + .should('be.visible') + .click({ force: true }) } export const moveFile = (fileName: string, dirPath: string) => { diff --git a/cypress/e2e/files/files-renaming.cy.ts b/cypress/e2e/files/files-renaming.cy.ts new file mode 100644 index 00000000000..e6dfc227f2a --- /dev/null +++ b/cypress/e2e/files/files-renaming.cy.ts @@ -0,0 +1,77 @@ +/** + * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import type { User } from '@nextcloud/cypress' +import { getRowForFile, triggerActionForFile } from './FilesUtils' + +const haveValidity = (validity: string | RegExp) => { + if (typeof validity === 'string') { + return (el: JQuery<HTMLElement>) => expect((el.get(0) as HTMLInputElement).validationMessage).to.equal(validity) + } + return (el: JQuery<HTMLElement>) => expect((el.get(0) as HTMLInputElement).validationMessage).to.match(validity) +} + +describe('files: Rename nodes', { testIsolation: true }, () => { + let user: User + + beforeEach(() => cy.createRandomUser().then(($user) => { + user = $user + + cy.uploadContent(user, new Blob([]), 'text/plain', '/file.txt') + cy.login(user) + cy.visit('/apps/files') + })) + + it('can rename a file', () => { + // All are visible by default + getRowForFile('file.txt').should('be.visible') + + triggerActionForFile('file.txt', 'rename') + + getRowForFile('file.txt') + .findByRole('textbox', { name: 'Filename' }) + .should('be.visible') + .type('{selectAll}other.txt') + .should(haveValidity('')) + .type('{enter}') + + // See it is renamed + getRowForFile('other.txt').should('be.visible') + }) + + /** + * If this test gets flaky than we have a problem: + * It means that the selection is not reliable set to the basename + */ + it('only selects basename of file', () => { + // All are visible by default + getRowForFile('file.txt').should('be.visible') + + triggerActionForFile('file.txt', 'rename') + + getRowForFile('file.txt') + .findByRole('textbox', { name: 'Filename' }) + .should('be.visible') + .should((el) => { + const input = el.get(0) as HTMLInputElement + expect(input.selectionStart).to.equal(0) + expect(input.selectionEnd).to.equal('file'.length) + }) + }) + + it('show validation error on file rename', () => { + // All are visible by default + getRowForFile('file.txt').should('be.visible') + + triggerActionForFile('file.txt', 'rename') + + getRowForFile('file.txt') + .findByRole('textbox', { name: 'Filename' }) + .should('be.visible') + .type('{selectAll}.htaccess') + // See validity + .should(haveValidity(/forbidden file or folder name/i)) + }) +}) diff --git a/package-lock.json b/package-lock.json index e72dd42d53f..6e01a8f1c0b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -20,7 +20,7 @@ "@nextcloud/capabilities": "^1.2.0", "@nextcloud/dialogs": "^5.3.7", "@nextcloud/event-bus": "^3.3.1", - "@nextcloud/files": "^3.6.0", + "@nextcloud/files": "^3.8.0", "@nextcloud/initial-state": "^2.2.0", "@nextcloud/l10n": "^3.1.0", "@nextcloud/logger": "^2.5.0", diff --git a/package.json b/package.json index 945d6f7ea08..5c844af5359 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,7 @@ "@nextcloud/capabilities": "^1.2.0", "@nextcloud/dialogs": "^5.3.7", "@nextcloud/event-bus": "^3.3.1", - "@nextcloud/files": "^3.6.0", + "@nextcloud/files": "^3.8.0", "@nextcloud/initial-state": "^2.2.0", "@nextcloud/l10n": "^3.1.0", "@nextcloud/logger": "^2.5.0", |