From 1ec8dd457780b71409114220437dc6a93399895f Mon Sep 17 00:00:00 2001 From: Elias Nahum Date: Mon, 13 Aug 2018 10:19:29 -0300 Subject: [PATCH] Fix video playback when filename contains spaces (#1997) * Fix video playback when filename contains spaces * Feedback review --- app/screens/image_preview/downloader.android.js | 9 +++++---- app/screens/image_preview/downloader.ios.js | 7 ++++--- app/screens/image_preview/image_preview.js | 13 +++++-------- app/screens/image_preview/video_preview.js | 5 +++-- app/utils/file.js | 8 ++++++++ package-lock.json | 6 +++--- package.json | 2 +- share_extension/ios/extension_post.js | 2 +- 8 files changed, 30 insertions(+), 22 deletions(-) diff --git a/app/screens/image_preview/downloader.android.js b/app/screens/image_preview/downloader.android.js index a61788b48..9a0ff9e24 100644 --- a/app/screens/image_preview/downloader.android.js +++ b/app/screens/image_preview/downloader.android.js @@ -17,7 +17,7 @@ import {Client4} from 'mattermost-redux/client'; import {DeviceTypes} from 'app/constants/'; import FormattedText from 'app/components/formatted_text'; -import {isDocument, isVideo} from 'app/utils/file'; +import {getLocalFilePathFromFile, isDocument, isVideo} from 'app/utils/file'; import {emptyFunction} from 'app/utils/general'; const {DOCUMENTS_PATH, VIDEOS_PATH} = DeviceTypes; @@ -89,7 +89,7 @@ export default class Downloader extends PureComponent { ToastAndroid.show(started, ToastAndroid.SHORT); onDownloadStart(); - const dest = `${RNFetchBlob.fs.dirs.DownloadDir}/${data.id}-${file.caption}`; + let dest = `${RNFetchBlob.fs.dirs.DownloadDir}/${data.id}-${file.caption}`; let downloadFile = true; if (data.localPath) { @@ -100,15 +100,16 @@ export default class Downloader extends PureComponent { await RNFetchBlob.fs.cp(data.localPath, dest); } } else if (isVideo(data)) { - const path = `${VIDEOS_PATH}/${data.id}-${file.caption}`; + const path = getLocalFilePathFromFile(VIDEOS_PATH, file); const exists = await RNFetchBlob.fs.exists(path); if (exists) { downloadFile = false; + dest = getLocalFilePathFromFile(RNFetchBlob.fs.dirs.DownloadDir, file); await RNFetchBlob.fs.cp(path, dest); } } else if (isDocument(data)) { - const path = `${DOCUMENTS_PATH}/${data.id}-${file.caption}`; + const path = getLocalFilePathFromFile(DOCUMENTS_PATH, file); const exists = await RNFetchBlob.fs.exists(path); if (exists) { diff --git a/app/screens/image_preview/downloader.ios.js b/app/screens/image_preview/downloader.ios.js index a7f584913..84612a267 100644 --- a/app/screens/image_preview/downloader.ios.js +++ b/app/screens/image_preview/downloader.ios.js @@ -13,6 +13,7 @@ import {Client4} from 'mattermost-redux/client'; import FormattedText from 'app/components/formatted_text'; import mattermostBucket from 'app/mattermost_bucket'; +import {getLocalFilePathFromFile} from 'app/utils/file'; import {emptyFunction} from 'app/utils/general'; import LocalConfig from 'assets/config'; @@ -23,12 +24,12 @@ export default class Downloader extends PureComponent { static propTypes = { deviceHeight: PropTypes.number.isRequired, deviceWidth: PropTypes.number.isRequired, + downloadPath: PropTypes.string, file: PropTypes.object.isRequired, onDownloadCancel: PropTypes.func, onDownloadSuccess: PropTypes.func, prompt: PropTypes.bool, show: PropTypes.bool, - downloadPath: PropTypes.string, saveToCameraRoll: PropTypes.bool, }; @@ -305,7 +306,7 @@ export default class Downloader extends PureComponent { } } - options.path = `${downloadPath}/${data.id}-${file.caption}`; + options.path = getLocalFilePathFromFile(downloadPath, file); } else { options.fileCache = true; options.appendExt = data.extension; @@ -356,7 +357,7 @@ export default class Downloader extends PureComponent { } catch (error) { // cancellation throws so we need to catch if (downloadPath) { - RNFetchBlob.fs.unlink(`${downloadPath}/${data.id}-${file.caption}`); + RNFetchBlob.fs.unlink(getLocalFilePathFromFile(downloadPath, file)); } if (error.message !== 'cancelled' && this.mounted) { this.showDownloadFailedAlert(); diff --git a/app/screens/image_preview/image_preview.js b/app/screens/image_preview/image_preview.js index 5bd403d2d..c7b50bdec 100644 --- a/app/screens/image_preview/image_preview.js +++ b/app/screens/image_preview/image_preview.js @@ -25,19 +25,17 @@ import Gallery from 'react-native-image-gallery'; import EventEmitter from 'mattermost-redux/utils/event_emitter'; -import {DeviceTypes} from 'app/constants/'; import FileAttachmentDocument from 'app/components/file_attachment_list/file_attachment_document'; import FileAttachmentIcon from 'app/components/file_attachment_list/file_attachment_icon'; -import {NavigationTypes, PermissionTypes} from 'app/constants'; -import {isDocument, isVideo} from 'app/utils/file'; +import ProgressiveImage from 'app/components/progressive_image'; +import {DeviceTypes, NavigationTypes, PermissionTypes} from 'app/constants'; +import {getLocalFilePathFromFile, isDocument, isVideo} from 'app/utils/file'; import {emptyFunction} from 'app/utils/general'; import {calculateDimensions} from 'app/utils/images'; import Downloader from './downloader'; import VideoPreview from './video_preview'; -import ProgressiveImage from 'app/components/progressive_image'; - const {VIDEOS_PATH} = DeviceTypes; const {View: AnimatedView} = Animated; const AnimatedSafeAreaView = Animated.createAnimatedComponent(SafeAreaView); @@ -455,11 +453,10 @@ export default class ImagePreview extends PureComponent { saveVideoIOS = () => { const file = this.getCurrentFile(); - const {data} = file; if (this.refs.downloader) { EventEmitter.emit(NavigationTypes.NAVIGATION_CLOSE_MODAL); - this.refs.downloader.saveVideo(`${VIDEOS_PATH}/${data.id}-${file.caption}`); + this.refs.downloader.saveVideo(getLocalFilePathFromFile(VIDEOS_PATH, file)); } }; @@ -540,7 +537,7 @@ export default class ImagePreview extends PureComponent { } if (isVideo(file.data)) { - const path = `${VIDEOS_PATH}/${file.data.id}-${file.caption}`; + const path = getLocalFilePathFromFile(VIDEOS_PATH, file); const exist = await RNFetchBlob.fs.exists(path); if (exist) { items.push({ diff --git a/app/screens/image_preview/video_preview.js b/app/screens/image_preview/video_preview.js index 75b4702d5..ba545585a 100644 --- a/app/screens/image_preview/video_preview.js +++ b/app/screens/image_preview/video_preview.js @@ -18,6 +18,7 @@ import EventEmitter from 'mattermost-redux/utils/event_emitter'; import VideoControls, {PLAYER_STATE} from 'app/components/video_controls'; import {DeviceTypes} from 'app/constants/'; +import {getLocalFilePathFromFile} from 'app/utils/file'; import Downloader from './downloader.ios'; @@ -70,7 +71,7 @@ export default class VideoPreview extends PureComponent { async initializeComponent() { const {file} = this.props; const prefix = Platform.OS === 'android' ? 'file:/' : ''; - const path = `${VIDEOS_PATH}/${file.data.id}-${file.caption}`; + const path = getLocalFilePathFromFile(VIDEOS_PATH, file); const exist = await RNFetchBlob.fs.exists(`${prefix}${path}`); if (exist) { @@ -90,7 +91,7 @@ export default class VideoPreview extends PureComponent { onDownloadSuccess = () => { const {file} = this.props; - const path = file.data.localPath || `${VIDEOS_PATH}/${file.data.id}-${file.caption}`; + const path = file.data.localPath || getLocalFilePathFromFile(VIDEOS_PATH, file); this.setState({showDownloader: false, path}); }; diff --git a/app/utils/file.js b/app/utils/file.js index 51e6fc4c0..fc6435c71 100644 --- a/app/utils/file.js +++ b/app/utils/file.js @@ -215,3 +215,11 @@ function populateMaps() { } }); } + +export function getLocalFilePathFromFile(dir, file) { + if (dir && file && file.caption && file.data && file.data.id) { + return `${dir}/${file.data.id}-${decodeURIComponent(file.caption).replace(/\s+/g, '-')}`; + } + + return null; +} diff --git a/package-lock.json b/package-lock.json index c44ff0ad8..53bc4c968 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14928,9 +14928,9 @@ } }, "react-native-video": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/react-native-video/-/react-native-video-2.0.0.tgz", - "integrity": "sha1-8z+m+35+PJOrV4eUTO/Vi/c1WGc=", + "version": "3.2.0", + "resolved": "https://registry.npmjs.org/react-native-video/-/react-native-video-3.2.0.tgz", + "integrity": "sha512-LLqyV9xK67FQTcQDpYruyRODlkQdE59uLExGoXjBngBHrf0q/R13yYaLk3G4CU2Bz+bi3cVzzI6E+q03eNeVYQ==", "requires": { "keymirror": "0.1.1", "prop-types": "^15.5.10" diff --git a/package.json b/package.json index 196c317a3..799440f74 100644 --- a/package.json +++ b/package.json @@ -52,7 +52,7 @@ "react-native-tableview": "2.1.0", "react-native-tooltip": "5.2.0", "react-native-vector-icons": "4.6.0", - "react-native-video": "2.0.0", + "react-native-video": "3.2.0", "react-native-youtube": "enahum/react-native-youtube#3f395b620ae4e05a3f1c6bdeef3a1158e78daadc", "react-navigation": "1.5.11", "react-redux": "5.0.7", diff --git a/share_extension/ios/extension_post.js b/share_extension/ios/extension_post.js index 396559147..0fff7983b 100644 --- a/share_extension/ios/extension_post.js +++ b/share_extension/ios/extension_post.js @@ -250,7 +250,7 @@ export default class ExtensionPost extends PureComponent { const fullPath = item.value; const filePath = decodeURIComponent(fullPath.replace('file://', '')); const fileSize = await RNFetchBlob.fs.stat(filePath); - const filename = fullPath.replace(/^.*[\\/]/, ''); + const filename = decodeURIComponent(fullPath.replace(/^.*[\\/]/, '')); const extension = filename.split('.').pop(); if (this.useBackgroundUpload) {