Skip to content

Commit

Permalink
fix: Load preview URL for download-disabled shares
Browse files Browse the repository at this point in the history 
This was possible on Nextcloud 30 and previous due to a "bug":
The `download` permission was simply not rejected for public shares,
just a `hide` flag was set on the public share.

Now the permissions are correctly set, so loading a preview is not possible.
The work-around is to allow previews when the correct header is set.

Signed-off-by: Ferdinand Thiessen <[email protected]>
  • Loading branch information
@susnux
susnux committed Sep 8, 2024
1 parent b6722ac commit ad15a91
Show file tree
Hide file tree
Showing 11 changed files with 162 additions and 275 deletions.
3 changes: 1 addition & 2 deletions cypress/e2e/download-forbidden.cy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ describe('Disable download button if forbidden', { testIsolation: true }, () =>
.should('contain', 'image1 .jpg')
})

// TODO: Fix no-download files on server
it.skip('See the image can be shown', () => {
it('See the image can be shown', () => {
cy.getFile('image1.jpg').should('be.visible')
cy.openFile('image1.jpg')
cy.get('body > .viewer').should('be.visible')
Expand Down
6 changes: 2 additions & 4 deletions cypress/e2e/sharing/download-share-disabled.cy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,16 +110,14 @@ describe(`Download ${fileName} in viewer`, function() {
cy.get('body > .viewer').should('be.visible')
})

// TODO: FIX DOWNLOAD DISABLED SHARES
it.skip('Does not see a loading animation', function() {
it('Does not see a loading animation', function() {
cy.get('body > .viewer', { timeout: 10000 })
.should('be.visible')
.and('have.class', 'modal-mask')
.and('not.have.class', 'icon-loading')
})

// TODO: FIX DOWNLOAD DISABLED SHARES
it.skip('See the title on the viewer header but not the Download nor the menu button', function() {
it('See the title on the viewer header but not the Download nor the menu button', function() {
cy.get('body > .viewer .modal-header__name').should('contain', 'image1.jpg')
cy.get('body a[download="image1.jpg"]').should('not.exist')
cy.get('body > .viewer .modal-header button.action-item__menutoggle').should('not.exist')
Expand Down
125 changes: 70 additions & 55 deletions src/components/Images.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,63 +28,63 @@
:fileid="fileid"
@close="onClose" />

<template v-else-if="data !== null">
<img v-if="!livePhotoCanBePlayed"
ref="image"
:alt="alt"
<IconImageBroken v-if="!data" :size="64" />

<img v-else-if="!livePhotoCanBePlayed"
ref="image"
:alt="alt"
:class="{
dragging,
loaded,
zoomed: zoomRatio > 1
}"
:src="data"
:style="imgStyle"
@error.capture.prevent.stop.once="onFail"
@load="updateImgSize"
@wheel.stop.prevent="updateZoom"
@dblclick.prevent="onDblclick"
@pointerdown.prevent="pointerDown"
@pointerup.prevent="pointerUp"
@pointermove.prevent="pointerMove">

<template v-else-if="livePhoto">
<video v-show="livePhotoCanBePlayed"
ref="video"
:class="{
dragging,
loaded,
zoomed: zoomRatio > 1
}"
:src="data"
:style="imgStyle"
@error.capture.prevent.stop.once="onFail"
@load="updateImgSize"
:playsinline="true"
:poster="data"
:src="livePhotoSrc"
preload="metadata"
@canplaythrough="doneLoadingLivePhoto"
@loadedmetadata="updateImgSize"
@wheel.stop.prevent="updateZoom"
@error.capture.prevent.stop.once="onFail"
@dblclick.prevent="onDblclick"
@pointerdown.prevent="pointerDown"
@pointerup.prevent="pointerUp"
@pointermove.prevent="pointerMove">

<template v-if="livePhoto">
<video v-show="livePhotoCanBePlayed"
ref="video"
:class="{
dragging,
loaded,
zoomed: zoomRatio > 1
}"
:style="imgStyle"
:playsinline="true"
:poster="data"
:src="livePhotoSrc"
preload="metadata"
@canplaythrough="doneLoadingLivePhoto"
@loadedmetadata="updateImgSize"
@wheel.stop.prevent="updateZoom"
@error.capture.prevent.stop.once="onFail"
@dblclick.prevent="onDblclick"
@pointerdown.prevent="pointerDown"
@pointerup.prevent="pointerUp"
@pointermove.prevent="pointerMove"
@ended="stopLivePhoto" />
<button v-if="width !== 0"
class="live-photo_play_button"
:style="{left: `calc(50% - ${width/2}px)`}"
:disabled="!livePhotoCanBePlayed"
:aria-description="t('viewer', 'Play the live photo')"
@click="playLivePhoto"
@pointerenter="playLivePhoto"
@focus="playLivePhoto"
@pointerleave="stopLivePhoto"
@blur="stopLivePhoto">
<PlayCircleOutline v-if="livePhotoCanBePlayed" />
<NcLoadingIcon v-else />
<!-- TRANSLATORS Label of the button used at the top left corner of live photos to play them -->
{{ t('viewer', 'LIVE') }}
</button>
</template>
@pointermove.prevent="pointerMove"
@ended="stopLivePhoto" />
<button v-if="width !== 0"
class="live-photo_play_button"
:style="{left: `calc(50% - ${width/2}px)`}"
:disabled="!livePhotoCanBePlayed"
:aria-description="t('viewer', 'Play the live photo')"
@click="playLivePhoto"
@pointerenter="playLivePhoto"
@focus="playLivePhoto"
@pointerleave="stopLivePhoto"
@blur="stopLivePhoto">
<PlayCircleOutline v-if="livePhotoCanBePlayed" />
<NcLoadingIcon v-else />
<!-- TRANSLATORS Label of the button used at the top left corner of live photos to play them -->
{{ t('viewer', 'LIVE') }}
</button>
</template>
</div>
</template>
Expand All @@ -93,6 +93,7 @@
import Vue from 'vue'
import AsyncComputed from 'vue-async-computed'
import PlayCircleOutline from 'vue-material-design-icons/PlayCircleOutline.vue'
import IconImageBroken from 'vue-material-design-icons/ImageBroken.vue'

import axios from '@nextcloud/axios'
import { basename } from '@nextcloud/paths'
Expand All @@ -102,13 +103,15 @@ import { NcLoadingIcon } from '@nextcloud/vue'
import ImageEditor from './ImageEditor.vue'
import { findLivePhotoPeerFromFileId } from '../utils/livePhotoUtils'
import { getDavPath } from '../utils/fileUtils'
import { getPreviewIfAny } from '../utils/previewUtils'

Vue.use(AsyncComputed)

export default {
name: 'Images',

components: {
IconImageBroken,
ImageEditor,
PlayCircleOutline,
NcLoadingIcon,
Expand Down Expand Up @@ -192,23 +195,35 @@ export default {

// Load the raw gif instead of the static preview
if (this.mime === 'image/gif') {
// if the source failed fallback to the preview
if (this.fallback) {
return this.previewPath
}
return this.src
}

// If there is no preview and we have a direct source
// load it instead
if (this.source && !this.hasPreview && !this.previewUrl) {
return this.source
// First try the preview if any
if (!this.fallback && this.previewPath) {
return this.previewPath
}

// If loading the preview failed once, let's load the original file
if (this.fallback) {
return this.src
}
return this.src
},

return this.previewPath
async previewPath() {
return await getPreviewIfAny({
...this.$attrs,
fileid: this.fileid,
filename: this.filename,
previewUrl: this.previewUrl,
hasPreview: this.hasPreview,
davPath: this.davPath,
etag: this.$attrs.etag,
})
},
},

watch: {
active(val, old) {
// the item was hidden before and is now the current view
Expand Down
15 changes: 13 additions & 2 deletions src/mixins/Mime.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,11 @@
*
*/
import debounce from 'debounce'
import PreviewUrl from '../mixins/PreviewUrl.js'
import parsePath from 'path-parse'
import { getDavPath } from '../utils/fileUtils.ts'

export default {
inheritAttrs: false,
mixins: [PreviewUrl],
props: {
// Is the current component shown
active: {
Expand Down Expand Up @@ -115,6 +114,18 @@ export default {
},

computed: {
/**
* Absolute dav remote path of the file
*
* @return {string}
*/
davPath() {
return getDavPath({
filename: this.filename,
basename: this.basename,
})
},

name() {
return parsePath(this.basename).name
},
Expand Down
73 changes: 0 additions & 73 deletions src/mixins/PreviewUrl.js
View file

This file was deleted.

49 changes: 16 additions & 33 deletions src/utils/fileUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ import { getLanguage } from '@nextcloud/l10n'
import { encodePath } from '@nextcloud/paths'
import camelcase from 'camelcase'

import { isNumber } from './numberUtil'

export interface FileInfo {
/** ID of the file (not unique if shared, use source instead) */
fileid?: number
Expand Down Expand Up @@ -95,13 +93,7 @@ export function sortCompare(fileInfo1: FileInfo, fileInfo2: FileInfo, key: strin
return 1
}

// if this is a number, let's sort by integer
if (isNumber(fileInfo1[key]) && isNumber(fileInfo2[key])) {
const result = Number(fileInfo1[key]) - Number(fileInfo2[key])
return asc ? result : -result
}

// else we sort by string, so let's sort directories first
// let's sort directories first
if (fileInfo1.type === 'directory' && fileInfo2.type !== 'directory') {
return -1
} else if (fileInfo1.type !== 'directory' && fileInfo2.type === 'directory') {
Expand All @@ -114,8 +106,8 @@ export function sortCompare(fileInfo1: FileInfo, fileInfo2: FileInfo, key: strin
}
// finally sort by name
return asc
? fileInfo1[key].localeCompare(fileInfo2[key], getLanguage(), { numeric: true })
: -fileInfo1[key].localeCompare(fileInfo2[key], getLanguage(), { numeric: true })
? String(fileInfo1[key]).localeCompare(fileInfo2[key], getLanguage(), { numeric: true })
: -String(fileInfo1[key]).localeCompare(fileInfo2[key], getLanguage(), { numeric: true })
}

/**
Expand All @@ -124,29 +116,20 @@ export function sortCompare(fileInfo1: FileInfo, fileInfo2: FileInfo, key: strin
* @param obj The stat response to convert
*/
export function genFileInfo(obj: FileStat): FileInfo {
const fileInfo = {}

Object.keys(obj).forEach(key => {
const data = obj[key]

// flatten object if any
if (!!data && typeof data === 'object' && !Array.isArray(data)) {
Object.assign(fileInfo, genFileInfo(data))
} else {
// format key and add it to the fileInfo
if (data === 'false') {
fileInfo[camelcase(key)] = false
} else if (data === 'true') {
fileInfo[camelcase(key)] = true
} else {
fileInfo[camelcase(key)] = isNumber(data)
? Number(data)
: data
}
}
})
const fileStat = {
...(obj.props ?? {}),
...obj,
props: undefined,
}

return fileInfo as FileInfo
const fileInfo = Object.entries(fileStat)
// Make property names camel case
.map(([key, value]) => [camelcase(key), value])
// Convert boolean - Numbers are already parsed by the WebDAV client
.map(([key, value]) => [key, ['true', 'false'].includes(value as never) ? value === 'true' : value])
// remove undefined properties
.filter(([, value]) => value !== undefined)
return Object.fromEntries(fileInfo)
}

/**
Expand Down
Loading

0 comments on commit ad15a91

Please sign in to comment.