Rewrite XMLHttpRequest/send-content-type-string.htm to comply with data: URI inheritance model

RESOLVED FIXED in Firefox 56

Status

()

P2
normal
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: ckerschb, Assigned: annevk)

Tracking

unspecified
mozilla56
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: [domsecurity-active])

Attachments

(1 attachment, 1 obsolete attachment)

Since you offered to rewrite the test I am assigning this one to you. For testing you can switch the pref |pref("security.data_uri.unique_opaque_origin", true);|

Thanks
(Reporter)

Updated

2 years ago
Assignee: nobody → annevk
Blocks: 1377244
Priority: -- → P2
Whiteboard: [domsecurity-active]
I just realized there would be 2 more tests that need similar rewriting:

> dom/nodes/Document-contentType/contentType/contenttype_datauri_01.html
> dom/nodes/Document-contentType/contentType/contenttype_datauri_02.html

Would you be willing to rewrite those two as well? Might fit into this bug :-)
(Assignee)

Comment 2

2 years ago
I created https://github.com/w3c/web-platform-tests/pull/6470. Still needs review from someone. I ended up not rewriting the latter two tests as they were specifically testing the data URL part. Equivalent scenarios without data URLs were already covered. So I removed them.
(In reply to Anne (:annevk) from comment #2)
> I created https://github.com/w3c/web-platform-tests/pull/6470. Still needs
> review from someone.

Thanks Anne. Can't we just land in our codebase and then upstream the changes?

> I ended up not rewriting the latter two tests as they
> were specifically testing the data URL part. Equivalent scenarios without
> data URLs were already covered. So I removed them.

That sounds good to me. But same here, can we remove them from our codebase then?
Flags: needinfo?(annevk)
(Assignee)

Comment 4

a year ago
If that's better for you feel free to apply https://patch-diff.githubusercontent.com/raw/w3c/web-platform-tests/pull/6470.patch to the Mozilla tree (it's already been reviewed by jdm) and maybe squash the commits. I don't have a local checkout at the moment. If that's the way we go I'll close the PR and say we're addressing it here.
Flags: needinfo?(annevk)
Created attachment 8883874 [details] [diff] [review]
bug_1377557.patch

As per comment 4, took the patch from Anne and applied it to our tree. Carrying over r+ from jdm.
(Reporter)

Updated

a year ago
Attachment #8883874 - Flags: review+

Comment 6

a year ago
Pushed by mozilla@christophkerschbaumer.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/401a95460411
Web platform tests update to comply with data: URI inheritance model: Update send-content-type-string.htm, contenttype_datauri_02.html and remove contenttype_datauri_01.html. r=jdm
Created attachment 8883890 [details] [diff] [review]
bug_1377557_followup.patch

These [1] are the only 3 occurences of the testfile we removed. Do we have to do anything else, or just remove the file from manifest.json?

[1] https://dxr.mozilla.org/mozilla-central/search?q=contenttype_datauri_01.html&redirect=false
Attachment #8883890 - Flags: review?(annevk)
(Assignee)

Comment 8

a year ago
Comment on attachment 8883890 [details] [diff] [review]
bug_1377557_followup.patch

Review of attachment 8883890 [details] [diff] [review]:
-----------------------------------------------------------------

Looks alright to me, but not 100% I'm qualified to review this.
Attachment #8883890 - Flags: review?(annevk) → review+
Comment on attachment 8883890 [details] [diff] [review]
bug_1377557_followup.patch

As discussed with jgraham on IRC, we should not manutally touch the manifest.json file, hence I ran |./mach wpt-manifest-update| and pushed the follow up to inbound. Here is the full IRC log:

ckerschb> Christoph Kerschbaumer padenot: if removing a test from wpt tests, is it enough to remove it from the manifest.json?
12:07 PM padenot: https://bugzilla.mozilla.org/show_bug.cgi?id=1377557#c8
12:08 PM 
<padenot> jgraham, ^ ?
12:10 PM ⇐ Conjuror and marcosc quit  ↔ cosmo0920 nipped out  
12:14 PM 
<jgraham> ckerschb: You shouldn't manually edit the manifest.json; you should just delete the file and run |mach wpt-manifest-update|
12:14 PM 
<ckerschb> Christoph Kerschbaumer jgraham: I see
12:15 PM jgraham: what happened is, that I already deleted the file and pushed to inbound
12:15 PM ⇐ breezykermo quit (lachlankerm@moz-2aj5hc.broadband.iol.cz) Ping timeout: 121 seconds
12:15 PM 
<jgraham> Assuming the test is wrong. If the test is right but we happen to fail it you should update the expectation metadata
12:15 PM 
<ckerschb> Christoph Kerschbaumer what do i do now?
12:15 PM 
<jgraham> ckerschb: Update the manifest using |mach wpt-manifest-update| and push a fixup commit to inbound
Attachment #8883890 - Attachment is obsolete: true

Comment 11

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/401a95460411
https://hg.mozilla.org/mozilla-central/rev/4d6404af7e75
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.