Skip to content

Commit ef4dc7d

Browse files
committed
fix: Load preview URL for download-disabled shares
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]>
1 parent dcb8705 commit ef4dc7d

13 files changed

+164
-208
lines changed

cypress/e2e/download-forbidden.cy.ts

+1-2
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,7 @@ describe('Disable download button if forbidden', { testIsolation: true }, () =>
3535
.should('contain', 'image1 .jpg')
3636
})
3737

38-
// TODO: Fix no-download files on server
39-
it.skip('See the image can be shown', () => {
38+
it('See the image can be shown', () => {
4039
cy.getFile('image1.jpg').should('be.visible')
4140
cy.openFile('image1.jpg')
4241
cy.get('body > .viewer').should('be.visible')

cypress/e2e/sharing/download-share-disabled.cy.ts

+2-4
Original file line numberDiff line numberDiff line change
@@ -93,16 +93,14 @@ describe(`Download ${fileName} in viewer`, function() {
9393
cy.get('body > .viewer').should('be.visible')
9494
})
9595

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

104-
// TODO: FIX DOWNLOAD DISABLED SHARES
105-
it.skip('See the title on the viewer header but not the Download nor the menu button', function() {
103+
it('See the title on the viewer header but not the Download nor the menu button', function() {
106104
cy.get('body > .viewer .modal-header__name').should('contain', 'image1.jpg')
107105
cy.get('body a[download="image1.jpg"]').should('not.exist')
108106
cy.get('body > .viewer .modal-header button.action-item__menutoggle').should('not.exist')

src/components/Images.vue

+70-55
Original file line numberDiff line numberDiff line change
@@ -11,63 +11,63 @@
1111
:fileid="fileid"
1212
@close="onClose" />
1313

14-
<template v-else-if="data !== null">
15-
<img v-if="!livePhotoCanBePlayed"
16-
ref="image"
17-
:alt="alt"
14+
<IconImageBroken v-if="!data" :size="64" />
15+
16+
<img v-else-if="!livePhotoCanBePlayed"
17+
ref="image"
18+
:alt="alt"
19+
:class="{
20+
dragging,
21+
loaded,
22+
zoomed: zoomRatio > 1
23+
}"
24+
:src="data"
25+
:style="imgStyle"
26+
@error.capture.prevent.stop.once="onFail"
27+
@load="updateImgSize"
28+
@wheel.stop.prevent="updateZoom"
29+
@dblclick.prevent="onDblclick"
30+
@pointerdown.prevent="pointerDown"
31+
@pointerup.prevent="pointerUp"
32+
@pointermove.prevent="pointerMove">
33+
34+
<template v-else-if="livePhoto">
35+
<video v-show="livePhotoCanBePlayed"
36+
ref="video"
1837
:class="{
1938
dragging,
2039
loaded,
2140
zoomed: zoomRatio > 1
2241
}"
23-
:src="data"
2442
:style="imgStyle"
25-
@error.capture.prevent.stop.once="onFail"
26-
@load="updateImgSize"
43+
:playsinline="true"
44+
:poster="data"
45+
:src="livePhotoSrc"
46+
preload="metadata"
47+
@canplaythrough="doneLoadingLivePhoto"
48+
@loadedmetadata="updateImgSize"
2749
@wheel.stop.prevent="updateZoom"
50+
@error.capture.prevent.stop.once="onFail"
2851
@dblclick.prevent="onDblclick"
2952
@pointerdown.prevent="pointerDown"
3053
@pointerup.prevent="pointerUp"
31-
@pointermove.prevent="pointerMove">
32-
33-
<template v-if="livePhoto">
34-
<video v-show="livePhotoCanBePlayed"
35-
ref="video"
36-
:class="{
37-
dragging,
38-
loaded,
39-
zoomed: zoomRatio > 1
40-
}"
41-
:style="imgStyle"
42-
:playsinline="true"
43-
:poster="data"
44-
:src="livePhotoSrc"
45-
preload="metadata"
46-
@canplaythrough="doneLoadingLivePhoto"
47-
@loadedmetadata="updateImgSize"
48-
@wheel.stop.prevent="updateZoom"
49-
@error.capture.prevent.stop.once="onFail"
50-
@dblclick.prevent="onDblclick"
51-
@pointerdown.prevent="pointerDown"
52-
@pointerup.prevent="pointerUp"
53-
@pointermove.prevent="pointerMove"
54-
@ended="stopLivePhoto" />
55-
<button v-if="width !== 0"
56-
class="live-photo_play_button"
57-
:style="{left: `calc(50% - ${width/2}px)`}"
58-
:disabled="!livePhotoCanBePlayed"
59-
:aria-description="t('viewer', 'Play the live photo')"
60-
@click="playLivePhoto"
61-
@pointerenter="playLivePhoto"
62-
@focus="playLivePhoto"
63-
@pointerleave="stopLivePhoto"
64-
@blur="stopLivePhoto">
65-
<PlayCircleOutline v-if="livePhotoCanBePlayed" />
66-
<NcLoadingIcon v-else />
67-
<!-- TRANSLATORS Label of the button used at the top left corner of live photos to play them -->
68-
{{ t('viewer', 'LIVE') }}
69-
</button>
70-
</template>
54+
@pointermove.prevent="pointerMove"
55+
@ended="stopLivePhoto" />
56+
<button v-if="width !== 0"
57+
class="live-photo_play_button"
58+
:style="{left: `calc(50% - ${width/2}px)`}"
59+
:disabled="!livePhotoCanBePlayed"
60+
:aria-description="t('viewer', 'Play the live photo')"
61+
@click="playLivePhoto"
62+
@pointerenter="playLivePhoto"
63+
@focus="playLivePhoto"
64+
@pointerleave="stopLivePhoto"
65+
@blur="stopLivePhoto">
66+
<PlayCircleOutline v-if="livePhotoCanBePlayed" />
67+
<NcLoadingIcon v-else />
68+
<!-- TRANSLATORS Label of the button used at the top left corner of live photos to play them -->
69+
{{ t('viewer', 'LIVE') }}
70+
</button>
7171
</template>
7272
</div>
7373
</template>
@@ -76,6 +76,7 @@
7676
import Vue from 'vue'
7777
import AsyncComputed from 'vue-async-computed'
7878
import PlayCircleOutline from 'vue-material-design-icons/PlayCircleOutline.vue'
79+
import IconImageBroken from 'vue-material-design-icons/ImageBroken.vue'
7980

8081
import axios from '@nextcloud/axios'
8182
import { basename } from '@nextcloud/paths'
@@ -85,13 +86,15 @@ import { NcLoadingIcon } from '@nextcloud/vue'
8586
import ImageEditor from './ImageEditor.vue'
8687
import { findLivePhotoPeerFromFileId } from '../utils/livePhotoUtils'
8788
import { getDavPath } from '../utils/fileUtils'
89+
import { getPreviewIfAny } from '../utils/previewUtils'
8890

8991
Vue.use(AsyncComputed)
9092

9193
export default {
9294
name: 'Images',
9395

9496
components: {
97+
IconImageBroken,
9598
ImageEditor,
9699
PlayCircleOutline,
97100
NcLoadingIcon,
@@ -175,23 +178,35 @@ export default {
175178

176179
// Load the raw gif instead of the static preview
177180
if (this.mime === 'image/gif') {
181+
// if the source failed fallback to the preview
182+
if (this.fallback) {
183+
return this.previewPath
184+
}
178185
return this.src
179186
}
180187

181-
// If there is no preview and we have a direct source
182-
// load it instead
183-
if (this.source && !this.hasPreview && !this.previewUrl) {
184-
return this.source
188+
// First try the preview if any
189+
if (!this.fallback && this.previewPath) {
190+
return this.previewPath
185191
}
186192

187193
// If loading the preview failed once, let's load the original file
188-
if (this.fallback) {
189-
return this.src
190-
}
194+
return this.src
195+
},
191196

192-
return this.previewPath
197+
async previewPath() {
198+
return await getPreviewIfAny({
199+
...this.$attrs,
200+
fileid: this.fileid,
201+
filename: this.filename,
202+
previewUrl: this.previewUrl,
203+
hasPreview: this.hasPreview,
204+
davPath: this.davPath,
205+
etag: this.$attrs.etag,
206+
})
193207
},
194208
},
209+
195210
watch: {
196211
active(val, old) {
197212
// the item was hidden before and is now the current view

src/mixins/Mime.js

+13-2
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,11 @@
33
* SPDX-License-Identifier: AGPL-3.0-or-later
44
*/
55
import debounce from 'debounce'
6-
import PreviewUrl from '../mixins/PreviewUrl.js'
76
import parsePath from 'path-parse'
7+
import { getDavPath } from '../utils/fileUtils.ts'
88

99
export default {
1010
inheritAttrs: false,
11-
mixins: [PreviewUrl],
1211
props: {
1312
// Is the current component shown
1413
active: {
@@ -98,6 +97,18 @@ export default {
9897
},
9998

10099
computed: {
100+
/**
101+
* Absolute dav remote path of the file
102+
*
103+
* @return {string}
104+
*/
105+
davPath() {
106+
return getDavPath({
107+
filename: this.filename,
108+
basename: this.basename,
109+
})
110+
},
111+
101112
name() {
102113
return parsePath(this.basename).name
103114
},

src/mixins/PreviewUrl.js

-56
This file was deleted.

src/utils/canDownload.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ import type { FileInfo } from './fileUtils'
1212
export function canDownload(fileInfo: FileInfo) {
1313
// TODO: This should probably be part of `@nextcloud/sharing`
1414
// check share attributes
15-
const shareAttributes = typeof fileInfo?.shareAttributes === 'string' ? JSON.parse(fileInfo.shareAttributes || '[]') : fileInfo?.shareAttributes
15+
const shareAttributes = typeof fileInfo?.shareAttributes === 'string'
16+
? JSON.parse(fileInfo.shareAttributes || '[]')
17+
: fileInfo?.shareAttributes
1618

1719
if (shareAttributes && shareAttributes.length > 0) {
1820
const downloadAttribute = shareAttributes.find(({ scope, key }) => scope === 'permissions' && key === 'download')

src/utils/fileUtils.ts

+17-34
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ import { getLanguage } from '@nextcloud/l10n'
99
import { encodePath } from '@nextcloud/paths'
1010
import camelcase from 'camelcase'
1111

12-
import { isNumber } from './numberUtil'
13-
1412
export interface FileInfo {
1513
/** ID of the file (not unique if shared, use source instead) */
1614
fileid?: number
@@ -33,7 +31,7 @@ export interface FileInfo {
3331
/** File type */
3432
type: 'directory'|'file'
3533
/** Attributes for file shares */
36-
shareAttributes?: string|Array<{value:boolean|string|number|null|object|Array<unknown>, key: string, scope: string}>
34+
shareAttributes?: string|Array<{ value: unknown, key: string, scope: string }>
3735

3836
// custom attributes not fetch from API
3937

@@ -78,13 +76,7 @@ export function sortCompare(fileInfo1: FileInfo, fileInfo2: FileInfo, key: strin
7876
return 1
7977
}
8078

81-
// if this is a number, let's sort by integer
82-
if (isNumber(fileInfo1[key]) && isNumber(fileInfo2[key])) {
83-
const result = Number(fileInfo1[key]) - Number(fileInfo2[key])
84-
return asc ? result : -result
85-
}
86-
87-
// else we sort by string, so let's sort directories first
79+
// let's sort directories first
8880
if (fileInfo1.type === 'directory' && fileInfo2.type !== 'directory') {
8981
return -1
9082
} else if (fileInfo1.type !== 'directory' && fileInfo2.type === 'directory') {
@@ -97,8 +89,8 @@ export function sortCompare(fileInfo1: FileInfo, fileInfo2: FileInfo, key: strin
9789
}
9890
// finally sort by name
9991
return asc
100-
? fileInfo1[key].localeCompare(fileInfo2[key], getLanguage(), { numeric: true })
101-
: -fileInfo1[key].localeCompare(fileInfo2[key], getLanguage(), { numeric: true })
92+
? String(fileInfo1[key]).localeCompare(fileInfo2[key], getLanguage(), { numeric: true })
93+
: -String(fileInfo1[key]).localeCompare(fileInfo2[key], getLanguage(), { numeric: true })
10294
}
10395

10496
/**
@@ -107,29 +99,20 @@ export function sortCompare(fileInfo1: FileInfo, fileInfo2: FileInfo, key: strin
10799
* @param obj The stat response to convert
108100
*/
109101
export function genFileInfo(obj: FileStat): FileInfo {
110-
const fileInfo = {}
111-
112-
Object.keys(obj).forEach(key => {
113-
const data = obj[key]
114-
115-
// flatten object if any
116-
if (!!data && typeof data === 'object' && !Array.isArray(data)) {
117-
Object.assign(fileInfo, genFileInfo(data))
118-
} else {
119-
// format key and add it to the fileInfo
120-
if (data === 'false') {
121-
fileInfo[camelcase(key)] = false
122-
} else if (data === 'true') {
123-
fileInfo[camelcase(key)] = true
124-
} else {
125-
fileInfo[camelcase(key)] = isNumber(data)
126-
? Number(data)
127-
: data
128-
}
129-
}
130-
})
102+
const fileStat = {
103+
...(obj.props ?? {}),
104+
...obj,
105+
props: undefined,
106+
}
131107

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

135118
/**

0 commit comments

Comments
 (0)