Skip to content

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

Closed
Ms2ger wants to merge 1 commit intomasterfrom
font-sniffing
Closed

Ignore the Content-Type header completely for @font-face.#13931
Ms2ger wants to merge 1 commit intomasterfrom
font-sniffing

Conversation

@Ms2ger
Copy link
Copy Markdown
Contributor

@Ms2ger Ms2ger commented Oct 26, 2016

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

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Oct 26, 2016
@Ms2ger
Copy link
Copy Markdown
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
Copy Markdown
Member

jdm commented Oct 26, 2016

Could you explain how these changes unblock #13714 and what they are intended to do?

@jdm jdm added S-awaiting-answer Someone asked a question that requires an answer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Oct 26, 2016
@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Nov 2, 2016
@Ms2ger
Copy link
Copy Markdown
Contributor Author

Ms2ger commented Nov 2, 2016

I added a long commit message that should hopefully help.

@Ms2ger Ms2ger removed the S-awaiting-answer Someone asked a question that requires an answer. label Nov 2, 2016
@jdm
Copy link
Copy Markdown
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 added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Nov 2, 2016
@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels 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
Copy Markdown
Contributor Author

Ms2ger commented Nov 2, 2016

#14024

@jdm
Copy link
Copy Markdown
Member

jdm commented Nov 2, 2016

@bors-servo: r+

@bors-servo
Copy link
Copy Markdown
Contributor

📌 Commit 4f02413 has been approved by jdm

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Nov 2, 2016
@jdm jdm mentioned this pull request Nov 2, 2016
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.
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Nov 2, 2016
@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Nov 2, 2016
@Ms2ger
Copy link
Copy Markdown
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
Copy Markdown
Contributor

📌 Commit 7c17529 has been approved by jdm

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Nov 2, 2016
@jdm
Copy link
Copy Markdown
Member

jdm commented Nov 2, 2016

@bors-servo: r-
Run cargo-update to update the lockfiles.

@jdm jdm added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Nov 2, 2016
bors-servo pushed a commit that referenced this pull request Nov 2, 2016
Move remaining users of the legacy networking stack to fetch.

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

☔ The latest upstream changes (presumably #13961) made this pull request unmergeable. Please resolve the merge conflicts.

@Ms2ger Ms2ger deleted the font-sniffing branch November 4, 2016 11:11
@Ms2ger Ms2ger mentioned this pull request Nov 7, 2016
31 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-needs-code-changes Changes have not yet been made that were requested by a reviewer.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants