1615897 - Blob URL should not ignore query or fragment

Status: RESOLVED FIXED

(Core :: DOM: File, defect, P3)

Description

[Tom S [:evilpie]]

There are a bunch of test failures at https://jgraham.github.io/wptdash/?bugComponent=core%3A%3Adom%3A+file, which are caused by us ignoring fragments and query parameters on blob: URLs. I think this could be fixed by modifying this code. The question is if that is going to cause other failures.

Comment 1

[Andrea Marchesini [:baku]]

Do you have a try-push result to see what breaks? Maybe it breaks invalid tests.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Presumably removing the fragment is still correct; the only question is what should happen for the query part, right?

Comment 3

[Tom S [:evilpie]]

It seems like #fragment is supposed to be ignored when fetching, but not for URL.revokeObjectURL. So just changing that one place doesn't work.

Comment 4

[Tom S [:evilpie]]

I submitted the incorrect patch to try anyway: https://treeherder.mozilla.org/#/jobs?repo=try&revision=83c029e9b82996e27e4a9ee4933318d09c98e886

Comment 5

[Boris Zbarsky [:bzbarsky]]

One plausible thing we could do:

1. Only strip fragment here, not query.
2. Have an explicit check for fragment at the entrypoint of URL.revokeObjectURL and no-op if it's present (like we would for any non-matching URL).

I think that will generally give the right behavior, right?

Comment 6

[Boris Zbarsky [:bzbarsky]]

Plus we'd want to make sure the specs make all this clear...

Comment 7

[Masatoshi Kimura [:emk]]

What part of the spec is unclear?
https://w3c.github.io/FileAPI/#blob-url-resolve
https://w3c.github.io/FileAPI/#dfn-revokeObjectURL

Comment 8

[Boris Zbarsky [:bzbarsky]]

Just to be clear, I have not had a close look at the spec. Those two links are quite clear, yes, and not what we do.

Internally we have a bunch of other callsites into this code and it's not obvious to me whether they correspond to spec operations or not.

Comment 9

[Jens Stutte [:jstutte]]

Tom, are you able to continue to work on this or should I triage to the team? Thanks

Comment 10

[Tom S [:evilpie]]

I can try implementing comment 5, but I can't promise that I will be able to dig into hundreds of failures when that doesn't work.

Comment 11

[Tom S [:evilpie]]

Attachment: Bug 1615897 - Blob URL should not ignore query parameter and revokeObjectURL additionally should not ignore fragments. r?bzbarsky

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7aa8f1610d72
Blob URL should not ignore query parameter and revokeObjectURL additionally should not ignore fragments. r=baku

Comment 13

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/7aa8f1610d72