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

File uploads are broken on latest Firefox Nightly #949

Closed
k80w opened this issue Jun 10, 2019 · 1 comment · Fixed by #952
Closed

File uploads are broken on latest Firefox Nightly #949

k80w opened this issue Jun 10, 2019 · 1 comment · Fixed by #952
Labels
Z-Community-PR Issue is solved by a community member's PR

Comments

@k80w
Copy link

k80w commented Jun 10, 2019

Under the latest version of Firefox Nightly, upon uploading a file, the following text is always uploaded in place of the files real contents

function stream() {
    [native code]
}

This seems to be caused by http-api.js:70:

const body = file.stream ? file.stream : file;

On other browsers, File.stream will be undefined; however, Firefox Nightly now adheres to the latest draft of the File API which defines a .stream() method, meaning this line is swapping the content of the file for this stream method and later stringifying and uploading that method.

The original purpose of this line seems to be to keep compatibility with older code, which was supposedly encouraged to set file.stream to the stream to be uploaded. I'm not sure if this is still necessary, especially considering this is apparently undocumented behavior. Deleting this line entirely solves the issue.

If it is necessary to keep compatibility, we could just ensure the file isn't a File before checking for file.stream.

This may be the same issue as #908, as the description of that issue sounds similar.

@turt2live
Copy link
Member

Highly related (by error): element-hq/element-web#9924

Going to raise this as a fire because it looks like we aren't handling files correctly at all.

@turt2live turt2live added the Z-Community-PR Issue is solved by a community member's PR label Jun 10, 2019
jryans added a commit that referenced this issue Jun 11, 2019
Modern browsers now expose a `stream` function on the Blob and File interfaces.
This conflicts with an older style of passing data to the `uploadContent` SDK
method, which supported supplying the data to upload in the `stream` property of
an object.

Since this old style is still in active use in the Matrix JS ecosystem, we
preserve the backwards compatibility for now by checking whether `stream` is a
function.

Fixes element-hq/element-web#9913
Fixes #949
jryans added a commit that referenced this issue Jun 11, 2019
Modern browsers now expose a `stream` function on the Blob and File interfaces.
This conflicts with an older style of passing data to the `uploadContent` SDK
method, which supported supplying the data to upload in the `stream` property of
an object.

Since this old style is still in active use in the Matrix JS ecosystem, we
preserve the backwards compatibility for now by checking whether `stream` is a
function.

This fix has been tested in Firefox Nightly (69), Firefox Release (67), Chrome
Canary (77), and Chrome Stable (75).

Fixes element-hq/element-web#9913
Fixes #949
@dbkr dbkr closed this as completed in #952 Jun 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
2 participants