-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Fix share extension handling for .HEIC files #68348
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
Changes from all commits
a6f17d7
6093743
2c391e7
298ecbb
11b2e55
db7d30f
c338df1
9fa5134
83b6c77
1bd6fb5
14040ab
8675751
2a33316
3b121dd
a1b15cf
e3ddc92
01a0cee
a9284f7
bade1c6
73f1c4b
f7b8011
486f243
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,10 +3,10 @@ import findLast from 'lodash/findLast'; | |
| import type {OnyxEntry} from 'react-native-onyx'; | ||
| import CONST from '@src/CONST'; | ||
| import ROUTES from '@src/ROUTES'; | ||
| import type {Transaction} from '@src/types/onyx'; | ||
| import type {ShareTempFile, Transaction} from '@src/types/onyx'; | ||
| import type {ReceiptError, ReceiptSource} from '@src/types/onyx/Transaction'; | ||
| import * as FileUtils from './fileDownload/FileUtils'; | ||
| import * as TransactionUtils from './TransactionUtils'; | ||
| import {isLocalFile as isLocalFileUtils, splitExtensionFromFileName} from './fileDownload/FileUtils'; | ||
| import {hasReceipt, hasReceiptSource, isFetchingWaypointsFromServer} from './TransactionUtils'; | ||
|
|
||
| type ThumbnailAndImageURI = { | ||
| image?: string; | ||
|
|
@@ -27,10 +27,10 @@ type ThumbnailAndImageURI = { | |
| * @param receiptFileName | ||
| */ | ||
| function getThumbnailAndImageURIs(transaction: OnyxEntry<Transaction>, receiptPath: ReceiptSource | null = null, receiptFileName: string | null = null): ThumbnailAndImageURI { | ||
| if (!TransactionUtils.hasReceipt(transaction) && !receiptPath && !receiptFileName) { | ||
| if (!hasReceipt(transaction) && !receiptPath && !receiptFileName) { | ||
| return {isEmptyReceipt: true}; | ||
| } | ||
| if (TransactionUtils.isFetchingWaypointsFromServer(transaction)) { | ||
| if (isFetchingWaypointsFromServer(transaction)) { | ||
| return {isThumbnail: true, isLocalFile: true}; | ||
| } | ||
| // If there're errors, we need to display them in preview. We can store many files in errors, but we just need to get the last one | ||
|
|
@@ -40,7 +40,7 @@ function getThumbnailAndImageURIs(transaction: OnyxEntry<Transaction>, receiptPa | |
| // filename of uploaded image or last part of remote URI | ||
| const filename = errors?.filename ?? transaction?.filename ?? receiptFileName ?? ''; | ||
| const isReceiptImage = Str.isImage(filename); | ||
| const hasEReceipt = !TransactionUtils.hasReceiptSource(transaction) && transaction?.hasEReceipt; | ||
| const hasEReceipt = !hasReceiptSource(transaction) && transaction?.hasEReceipt; | ||
| const isReceiptPDF = Str.isPDF(filename); | ||
|
|
||
| if (hasEReceipt) { | ||
|
|
@@ -60,11 +60,15 @@ function getThumbnailAndImageURIs(transaction: OnyxEntry<Transaction>, receiptPa | |
| return {thumbnail: `${path.substring(0, path.length - 4)}.jpg.1024.jpg`, image: path, filename}; | ||
| } | ||
|
|
||
| const isLocalFile = FileUtils.isLocalFile(path); | ||
| const {fileExtension} = FileUtils.splitExtensionFromFileName(filename); | ||
| const isLocalFile = isLocalFileUtils(path); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did we import this as
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. due to a naming conflict with a previously existing variable with the name |
||
| const {fileExtension} = splitExtensionFromFileName(filename); | ||
| return {isThumbnail: true, fileExtension: Object.values(CONST.IOU.FILE_TYPES).find((type) => type === fileExtension), image: path, isLocalFile, filename}; | ||
| } | ||
|
|
||
| const shouldValidateFile = (file: ShareTempFile | undefined) => { | ||
| return file?.mimeType === CONST.SHARE_FILE_MIMETYPE.HEIC || file?.mimeType === CONST.SHARE_FILE_MIMETYPE.IMG; | ||
| }; | ||
|
|
||
| // eslint-disable-next-line import/prefer-default-export | ||
| export {getThumbnailAndImageURIs}; | ||
| export {getThumbnailAndImageURIs, shouldValidateFile}; | ||
| export type {ThumbnailAndImageURI}; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ import {getFileName, readFileAsync} from '@libs/fileDownload/FileUtils'; | |
| import Navigation from '@libs/Navigation/Navigation'; | ||
| import type {ShareNavigatorParamList} from '@libs/Navigation/types'; | ||
| import {getReportDisplayOption} from '@libs/OptionsListUtils'; | ||
| import {shouldValidateFile} from '@libs/ReceiptUtils'; | ||
| import {getReportOrDraftReport, isDraftReport} from '@libs/ReportUtils'; | ||
| import NotFoundPage from '@pages/ErrorPage/NotFoundPage'; | ||
| import variables from '@styles/variables'; | ||
|
|
@@ -47,15 +48,22 @@ function ShareDetailsPage({ | |
| const {translate} = useLocalize(); | ||
| const [unknownUserDetails] = useOnyx(ONYXKEYS.SHARE_UNKNOWN_USER_DETAILS, {canBeMissing: true}); | ||
| const [currentAttachment] = useOnyx(ONYXKEYS.SHARE_TEMP_FILE, {canBeMissing: true}); | ||
| const [validatedFile] = useOnyx(ONYXKEYS.VALIDATED_FILE_OBJECT, {canBeMissing: true}); | ||
|
|
||
| const [reportAttributesDerived] = useOnyx(ONYXKEYS.DERIVED.REPORT_ATTRIBUTES, {canBeMissing: true, selector: (val) => val?.reports}); | ||
| const isTextShared = currentAttachment?.mimeType === 'txt'; | ||
| const isTextShared = currentAttachment?.mimeType === CONST.SHARE_FILE_MIMETYPE.TXT; | ||
| const shouldUsePreValidatedFile = shouldValidateFile(currentAttachment); | ||
| const [message, setMessage] = useState(isTextShared ? (currentAttachment?.content ?? '') : ''); | ||
| const [errorTitle, setErrorTitle] = useState<string | undefined>(undefined); | ||
| const [errorMessage, setErrorMessage] = useState<string | undefined>(undefined); | ||
|
|
||
| const report: OnyxEntry<ReportType> = getReportOrDraftReport(reportOrAccountID); | ||
| const displayReport = useMemo(() => getReportDisplayOption(report, unknownUserDetails, reportAttributesDerived), [report, unknownUserDetails, reportAttributesDerived]); | ||
|
|
||
| const fileSource = shouldUsePreValidatedFile ? (validatedFile?.uri ?? '') : (currentAttachment?.content ?? ''); | ||
| const validateFileName = shouldUsePreValidatedFile ? getFileName(validatedFile?.uri ?? CONST.ATTACHMENT_IMAGE_DEFAULT_NAME) : getFileName(currentAttachment?.content ?? ''); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The direct access of filename check caused this issue - #74542 |
||
| const fileType = shouldUsePreValidatedFile ? (validatedFile?.type ?? CONST.SHARE_FILE_MIMETYPE.JPEG) : (currentAttachment?.mimeType ?? ''); | ||
|
|
||
| useEffect(() => { | ||
| if (!currentAttachment?.content || errorTitle) { | ||
| return; | ||
|
|
@@ -89,10 +97,8 @@ function ShareDetailsPage({ | |
| const currentUserID = getCurrentUserAccountID(); | ||
| const shouldShowAttachment = !isTextShared; | ||
|
|
||
| const fileName = currentAttachment?.content.split('/').pop(); | ||
|
|
||
| const handleShare = () => { | ||
| if (!currentAttachment) { | ||
| if (!currentAttachment || (shouldUsePreValidatedFile && !validatedFile)) { | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -104,8 +110,8 @@ function ShareDetailsPage({ | |
| } | ||
|
|
||
| readFileAsync( | ||
| currentAttachment.content, | ||
| getFileName(currentAttachment.content), | ||
| fileSource, | ||
| validateFileName, | ||
| (file) => { | ||
| if (isDraft) { | ||
| openReport( | ||
|
|
@@ -126,7 +132,7 @@ function ShareDetailsPage({ | |
| Navigation.navigate(routeToNavigate, {forceReplace: true}); | ||
| }, | ||
| () => {}, | ||
| currentAttachment.mimeType, | ||
| fileType, | ||
| ); | ||
| }; | ||
|
|
||
|
|
@@ -196,14 +202,14 @@ function ShareDetailsPage({ | |
| </View> | ||
| <SafeAreaView> | ||
| <AttachmentModal | ||
| headerTitle={fileName} | ||
| source={currentAttachment?.content} | ||
| originalFileName={fileName} | ||
| headerTitle={validateFileName} | ||
| source={fileSource} | ||
| originalFileName={validateFileName} | ||
| fallbackSource={FallbackAvatar} | ||
| > | ||
| {({show}) => ( | ||
| <AttachmentPreview | ||
| source={currentAttachment?.content ?? ''} | ||
| source={fileSource ?? ''} | ||
| aspectRatio={currentAttachment?.aspectRatio} | ||
| onPress={show} | ||
| onLoadError={() => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -5,16 +5,20 @@ | |||||
| import ScreenWrapper from '@components/ScreenWrapper'; | ||||||
| import TabNavigatorSkeleton from '@components/Skeletons/TabNavigatorSkeleton'; | ||||||
| import TabSelector from '@components/TabSelector/TabSelector'; | ||||||
| import useFilesValidation from '@hooks/useFilesValidation'; | ||||||
| import useLocalize from '@hooks/useLocalize'; | ||||||
| import useOnyx from '@hooks/useOnyx'; | ||||||
| import useThemeStyles from '@hooks/useThemeStyles'; | ||||||
| import {addTempShareFile, clearShareData} from '@libs/actions/Share'; | ||||||
| import {addTempShareFile, addValidatedShareFile, clearShareData} from '@libs/actions/Share'; | ||||||
| import {canUseTouchScreen} from '@libs/DeviceCapabilities'; | ||||||
| import {splitExtensionFromFileName, validateImageForCorruption} from '@libs/fileDownload/FileUtils'; | ||||||
| import Navigation from '@libs/Navigation/Navigation'; | ||||||
| import OnyxTabNavigator, {TopTab} from '@libs/Navigation/OnyxTabNavigator'; | ||||||
| import {shouldValidateFile} from '@libs/ReceiptUtils'; | ||||||
| import ShareActionHandler from '@libs/ShareActionHandlerModule'; | ||||||
| import type {FileObject} from '@pages/media/AttachmentModalScreen/types'; | ||||||
| import CONST from '@src/CONST'; | ||||||
| import ONYXKEYS from '@src/ONYXKEYS'; | ||||||
| import ROUTES from '@src/ROUTES'; | ||||||
| import type {ShareTempFile} from '@src/types/onyx'; | ||||||
| import getFileSize from './getFileSize'; | ||||||
|
|
@@ -33,6 +37,25 @@ | |||||
| } | ||||||
|
|
||||||
| function ShareRootPage() { | ||||||
| const [currentAttachment] = useOnyx(ONYXKEYS.SHARE_TEMP_FILE, {canBeMissing: true}); | ||||||
|
|
||||||
| const {validateFiles} = useFilesValidation(addValidatedShareFile); | ||||||
| const isTextShared = currentAttachment?.mimeType === 'txt'; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| const validateFileIfNecessary = (file: ShareTempFile) => { | ||||||
| if (!file || isTextShared || !shouldValidateFile(file)) { | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| validateFiles([ | ||||||
| { | ||||||
| name: file.id, | ||||||
| uri: file.content, | ||||||
| type: file.mimeType, | ||||||
| }, | ||||||
| ]); | ||||||
| }; | ||||||
sumo-slonik marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
|
||||||
| const appState = useRef(AppState.currentState); | ||||||
| const [isFileReady, setIsFileReady] = useState(false); | ||||||
|
|
||||||
|
|
@@ -95,13 +118,14 @@ | |||||
| } else { | ||||||
| setIsFileScannable(false); | ||||||
| } | ||||||
| validateFileIfNecessary(tempFile); | ||||||
| setIsFileReady(true); | ||||||
| } | ||||||
|
|
||||||
| addTempShareFile(tempFile); | ||||||
| } | ||||||
| }); | ||||||
| }, [receiptFileFormats, shareFileMimeTypes, translate, errorTitle]); | ||||||
|
|
||||||
| useEffect(() => { | ||||||
| const subscription = AppState.addEventListener('change', (nextAppState) => { | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need this btw? What scenario does the
validatedFile?.urinot exist?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If an error occurs during validation, or if we are not in a case where validation should be performed, this field will be empty. That’s why we need to guard against such cases.