Skip to content

[css-fonts-4] Use “create a potential-CORS request” #5838

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 2, 2021
Merged

[css-fonts-4] Use “create a potential-CORS request” #5838

merged 1 commit into from
Mar 2, 2021

Conversation

sideshowbarker
Copy link
Member

@sideshowbarker sideshowbarker commented Jan 6, 2021 •
edited
Loading

This change makes the CSS Fonts spec reference the “create a potential-CORS request” algorithm from the HTML spec.

Otherwise, without this change, the CSS Fonts spec references “potentially CORS-enabled fetch”, which is an obsolete algorithm that no longer exists either in the Fetch spec nor in HTML.

Related: whatwg/html#6265

@sideshowbarker sideshowbarker force-pushed the sideshowbarker/fonts-use-create-a-potential-cors-request branch from 75e4f4a to f75d37e Compare January 6, 2021 03:47
@sideshowbarker sideshowbarker mentioned this pull request Jan 6, 2021
Closed
@w3cbot
Copy link

w3cbot commented Jan 6, 2021

sideshowbarker marked as non substantive for IPR from ash-nazg.

@sideshowbarker sideshowbarker force-pushed the sideshowbarker/fonts-use-create-a-potential-cors-request branch from f75d37e to 3b45813 Compare January 6, 2021 04:12
@annevk
Copy link
Member

annevk commented Jan 6, 2021

Why can this not create a request directly? HTML's abstraction is mostly useful if you have a crossorigin attribute, which I doubt is the case here?

@sideshowbarker
Copy link
Member Author

Why can this not create a request directly? HTML's abstraction is mostly useful if you have a crossorigin attribute, which I doubt is the case here?

Good point. I'll rewrite it

@svgeesus
Copy link
Contributor

svgeesus commented Jan 7, 2021

Why can this not create a request directly? HTML's abstraction is mostly useful if you have a crossorigin attribute, which I doubt is the case here?

It is not the case for the CSS url() syntax, no.

It will be the case for the planned replacement for url() which among other things will deal with origin.

@svgeesus
Copy link
Contributor

svgeesus commented Jan 7, 2021

Good point. I'll rewrite it

Will wait for your rewrite before merging.

@svgeesus svgeesus self-assigned this Jan 7, 2021
@svgeesus svgeesus added the css-fonts-4 Current Work label Jan 7, 2021
@annevk
Copy link
Member

annevk commented Jan 8, 2021

@svgeesus even in that case I doubt it would have an actual HTML attribute state, which is what that algorithm expects.

@syncbot syncbot force-pushed the sideshowbarker/fonts-use-create-a-potential-cors-request branch from 3b45813 to 75e4f4a Compare January 13, 2021 22:38
@svgeesus
Copy link
Contributor

@sideshowbarker

Will wait for your rewrite before merging.

Is the force-push the rewrite, or should I still wait?

@sideshowbarker sideshowbarker marked this pull request as draft January 24, 2021 11:33
@sideshowbarker
Copy link
Member Author

Hi @svgeesus

Is the force-push the rewrite, or should I still wait?

The force-push is not the rewrite, so please wait. (Sorry for not having gotten back to this yet; I’ll try to do it early this week.)

@sideshowbarker sideshowbarker self-assigned this Jan 24, 2021
@triple-underscore
Copy link

Some of other CSS modules also use “potentially CORS-enabled fetch” algorithm.
Specifically, I found it in css-masking, css-shapes and filter-effects (and css-font-3).
It would be nice if they are updated together.

@sideshowbarker
Copy link
Member Author

Some of other CSS modules also use “potentially CORS-enabled fetch” algorithm.
Specifically, I found it in css-masking, css-shapes and filter-effects (and css-font-3).
It would be nice if they are updated together.

OK, thanks for the heads-up — I’ll update those too

Base automatically changed from master to main February 2, 2021 19:45
@sideshowbarker
[css-fonts-4] Call Fetch directly for @font-face loads
17da2bb 
This change makes the CSS Fonts spec call directly into the Fetch spec
when specifying the requirements for loading fonts from @font-face rules.

Otherwise, without this change, the CSS Fonts spec references
“potentially CORS-enabled fetch”, which is an obsolete algorithm that no
longer exists either in the Fetch spec nor in HTML.
@sideshowbarker sideshowbarker force-pushed the sideshowbarker/fonts-use-create-a-potential-cors-request branch from 75e4f4a to 17da2bb Compare February 27, 2021 12:34
@sideshowbarker sideshowbarker marked this pull request as ready for review February 27, 2021 12:34
@sideshowbarker
Copy link
Member Author

OK, I’ve amended the commit and this is now ready for re-review

@sideshowbarker
Copy link
Member Author

Some of other CSS modules also use “potentially CORS-enabled fetch” algorithm.
Specifically, I found it in css-masking, css-shapes and filter-effects (and css-font-3).
It would be nice if they are updated together.

I’d prefer to get this one change right first — rather than trying to include patches to multiple other specs in this PR. Once we have the language in this PR right, the language can be re-used for patches to the other specs.

@syncbot syncbot force-pushed the sideshowbarker/fonts-use-create-a-potential-cors-request branch 2 times, most recently from 75e4f4a to 17da2bb Compare February 28, 2021 19:17
svgeesus
svgeesus approved these changes Mar 2, 2021
View reviewed changes
Copy link
Contributor

@svgeesus svgeesus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent, thanks

@svgeesus svgeesus merged commit 4fb63d3 into main Mar 2, 2021
@sideshowbarker sideshowbarker deleted the sideshowbarker/fonts-use-create-a-potential-cors-request branch March 2, 2021 16:11
@sideshowbarker sideshowbarker mentioned this pull request Mar 4, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@svgeesus svgeesus svgeesus approved these changes

@annevk annevk Awaiting requested review from annevk

Assignees

@sideshowbarker sideshowbarker

@svgeesus svgeesus

Labels
css-fonts-4 Current Work
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@sideshowbarker @w3cbot @annevk @svgeesus @triple-underscore