Skip to content

/ servo

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

Ignore the Content-Type header completely for @font-face. #13931

Closed
wants to merge 1 commit into from
Closed

Ignore the Content-Type header completely for @font-face. #13931

wants to merge 1 commit into from

Conversation

@Ms2ger
Copy link
Contributor

Ms2ger commented Oct 26, 2016 •
edited by larsbergstrom

This matches the previous default (network.mime.sniff off) behaviour in all
but one case: we will now accept a font without a Content-Type header, which
would previously have been ignored.

This change is Reviewable
@Ms2ger
Copy link
Contributor Author

Ms2ger commented Oct 26, 2016

This unblocks #13714.

r? @jdm
@highfive highfive assigned jdm and unassigned cbrewster Oct 26, 2016
@jdm
Copy link
Member

jdm commented Oct 26, 2016

Could you explain how these changes unblock #13714 and what they are intended to do?
@Ms2ger Ms2ger force-pushed the font-sniffing branch from 27babfb to 3d7d9b9 Nov 2, 2016
@Ms2ger
Copy link
Contributor Author

Ms2ger commented Nov 2, 2016

I added a long commit message that should hopefully help.
@Ms2ger Ms2ger force-pushed the font-sniffing branch from 3d7d9b9 to 0017c50 Nov 2, 2016
@jdm
Copy link
Member

jdm commented Nov 2, 2016

As discussed on IRC, I would like to see either the test removed along with all code that deals with the content type of font responses (along with an issue filed about implementing sniffing for fetch), or implementing sniffing for fetch properly.
@jdm jdm mentioned this pull request Nov 2, 2016
Merged
@Ms2ger Ms2ger force-pushed the font-sniffing branch from 0017c50 to 4f02413 Nov 2, 2016
@Ms2ger Ms2ger changed the title Simplify the Content-Type handling for @font-face. Ignore the Content-Type header completely for @font-face. Nov 2, 2016
@Ms2ger
Copy link
Contributor Author

Ms2ger commented Nov 2, 2016

#14024
@jdm
Copy link
Member

jdm commented Nov 2, 2016

@bors-servo: r+
@bors-servo
Copy link
Contributor

bors-servo commented Nov 2, 2016

📌 Commit 4f02413 has been approved by jdm
@jdm jdm mentioned this pull request Nov 2, 2016
Open
@Ms2ger
Ignore the Content-Type header completely for @font-face.
Loading status checks…
7c17529 
This matches the previous default (network.mime.sniff off) behaviour in all
but one case: we will now accept a font without a `Content-Type` header, which
would previously have been ignored.
@Ms2ger Ms2ger force-pushed the font-sniffing branch from 4f02413 to 7c17529 Nov 2, 2016
@Ms2ger
Copy link
Contributor Author

Ms2ger commented Nov 2, 2016

Also removed the now-unused dependency on the mime crate.

@bors-servo r=jdm
@bors-servo
Copy link
Contributor

bors-servo commented Nov 2, 2016

📌 Commit 7c17529 has been approved by jdm
@jdm
Copy link
Member

jdm commented Nov 2, 2016

@bors-servo: r-
Run cargo-update to update the lockfiles.
bors-servo added a commit that referenced this pull request Nov 2, 2016
@bors-servo
Auto merge of #13961 - servo:sync-fetch, r=jdm
Loading status checks…
56f4a69 
Move remaining users of the legacy networking stack to fetch.

Fixes #13931.
Fixes #13714.
@bors-servo
Copy link
Contributor

bors-servo commented Nov 2, 2016

☔ The latest upstream changes (presumably #13961) made this pull request unmergeable. Please resolve the merge conflicts.
@Ms2ger Ms2ger deleted the font-sniffing branch Nov 4, 2016
@Ms2ger Ms2ger mentioned this pull request Nov 7, 2016
Closed
31 of 31 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jdm jdm

Labels
S-needs-code-changes
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
@Ms2ger @jdm @bors-servo @highfive @cbrewster
You can’t perform that action at this time.