Skip to content
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

Fix viewer's "Export -> Save as..." with Firefox. #1626

Merged
merged 1 commit into from
Feb 3, 2017
Merged

Fix viewer's "Export -> Save as..." with Firefox. #1626

merged 1 commit into from
Feb 3, 2017

Conversation

XhmikosR
Copy link
Contributor

@XhmikosR XhmikosR commented Feb 3, 2017

For some weird reason, Firefox needs document.body.appendChild(a) too.

Fixes #1574.

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Feb 3, 2017

Only thing I wonder is if we should call removeChild too or not. Any feedback is appreciated.

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Feb 3, 2017

I added a new patch which also removes the node from DOM and changes the setTimeout's delay to 0 which works the same for FF and Chrome.

@@ -361,9 +361,12 @@ class LighthouseViewerReport extends LighthouseReport {
const a = document.createElement('a');
a.download = `${filename}${ext}`;
a.href = URL.createObjectURL(blob);
document.body.appendChild(a);
Copy link
Contributor

Choose a reason for hiding this comment

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

document.body.appendChild(a); // Firefox requires anchor be in the DOM.

setTimeout(_ => URL.revokeObjectURL(a.href), 500); // cleanup.
// cleanup.
document.body.removeChild(a);
setTimeout(_ => URL.revokeObjectURL(a.href), 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

We may want to keep the 500ms delay just to be on the safe side.

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Feb 3, 2017

@ebidel: done. I kept the interval to 500, even though it seems unneeded from my tests.

For some weird reason, Firefox needs `document.body.appendChild(a)` too.
Copy link
Contributor

@ebidel ebidel left a comment

Choose a reason for hiding this comment

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

Awesome. Thanks again.

@ebidel ebidel merged commit 5de65b5 into GoogleChrome:master Feb 3, 2017
@XhmikosR XhmikosR deleted the viewer-export-as-ff branch February 3, 2017 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants