Skip to content

fix(trace): Allow zip files of other mime types on paste #37907

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

fix(trace): Allow zip files of other mime types on paste #37907

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 14 additions & 18 deletions packages/trace-viewer/src/ui/workbenchLoader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,50 +23,48 @@ import { DialogToolbarButton } from '@web/components/dialogToolbarButton';
import { Dialog } from '@web/shared/dialog';
import { DefaultSettingsView } from './defaultSettingsView';
import { TraceModelContext } from './traceModelContext';
import { getMimeTypeForPath } from '@isomorphic/mimeType';

export const WorkbenchLoader: React.FunctionComponent<{
}> = () => {
const [isServer, setIsServer] = React.useState<boolean>(false);
const [traceURL, setTraceURL] = React.useState<string>();
const [uploadedTraceName, setUploadedTraceName] = React.useState<string>();
const [model, setModel] = React.useState<MultiTraceModel>(emptyModel);
const [progress, setProgress] = React.useState<{ done: number, total: number }>({ done: 0, total: 0 });
const [dragOver, setDragOver] = React.useState<boolean>(false);
const [processingErrorMessage, setProcessingErrorMessage] = React.useState<string | null>(null);
const [fileForLocalModeError, setFileForLocalModeError] = React.useState<string | null>(null);
const [showProgressDialog, setShowProgressDialog] = React.useState<boolean>(false);

const processTraceFiles = React.useCallback((files: FileList) => {
const url = new URL(window.location.href);
if (!files.length)
const processTraceFiles = React.useCallback((files?: FileList | null) => {
if (!files?.length)
return;
const file = files.item(0)!;

// Do best effort to select the first valid trace valid, if not, rely on error reporting of sw
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pasting multiple files looks like a user error, we can bail sooner or leave it as is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It definitely is a user error. However, why not try to find/load the valid trace file from this fileslist? I think that's preferable over not showing anything.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this regresses users that renamed their traces to foo.trace, what do we do to those?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.
However, in this case we just try to find the first .zip file, if not found it will just pick the first one and send it to the server to handle (possibly returning an error). So, processing will just work fine for them. It is just that those users don't 'benefit' from this 'best effort'

// Zip files may have different mime types on different operating systems, so rely on filename instead
const file = Array.from(files).find(file => getMimeTypeForPath(file.name) === 'application/zip') ?? files[0];
const blobTraceURL = URL.createObjectURL(file);

const url = new URL(window.location.href);
url.searchParams.append('trace', blobTraceURL);
const href = url.toString();
// Snapshot loaders will inherit the trace url from the query parameters,
// so set it here.
window.history.pushState({}, '', href);
setTraceURL(blobTraceURL);
setUploadedTraceName(file.name);
setDragOver(false);
setProcessingErrorMessage(null);
}, []);

React.useEffect(() => {
const listener = async (e: ClipboardEvent) => {
if (!e.clipboardData?.files.length)
return;
for (const file of e.clipboardData.files) {
if (file.type !== 'application/zip')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd just add application/x-zip-compressed for old Windows systems.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not just for old windows systems, I am experiencing this mime-type behaviour on Windows 11.
But then, there are probably other cases we need to consider depending on browser/operating system (https://mime-type.com/file-extension/zip/). So checking file extension seems easiest and most reliable here?

return;
}
e.preventDefault();
processTraceFiles(e.clipboardData.files);
processTraceFiles(e.clipboardData?.files);
};
document.addEventListener('paste', listener);
return () => document.removeEventListener('paste', listener);
});

React.useEffect(() => {
const listener = (e: MessageEvent) => {
const { method, params } = e.data;
Expand All @@ -90,11 +88,9 @@ export const WorkbenchLoader: React.FunctionComponent<{
processTraceFiles(event.dataTransfer.files);
}, [processTraceFiles]);

const handleFileInputChange = React.useCallback((event: any) => {
const handleFileInputChange = React.useCallback((event: Event) => {
event.preventDefault();
if (!event.target.files)
return;
processTraceFiles(event.target.files);
processTraceFiles((event.target as HTMLInputElement).files);
}, [processTraceFiles]);

React.useEffect(() => {
Expand Down Expand Up @@ -157,7 +153,7 @@ export const WorkbenchLoader: React.FunctionComponent<{
navigator.serviceWorker.removeEventListener('message', swListener);
}
})();
}, [isServer, traceURL, uploadedTraceName]);
}, [isServer, traceURL]);

const showLoading = progress.done !== progress.total && progress.total !== 0 && !processingErrorMessage;

Expand Down