Skip to content
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

Don't allow CSS escapes? #97

Closed
zcorpan opened this issue Feb 4, 2014 · 6 comments
Closed

Don't allow CSS escapes? #97

zcorpan opened this issue Feb 4, 2014 · 6 comments

Comments

@zcorpan
Copy link

zcorpan commented Feb 4, 2014

@annevk points out that the new syntax appears to support CSS escapes e.g. in the url in srcset. I'm not sure that is intentional or desirable and it doesn't match "old" srcset.

@zcorpan
Copy link
Author

zcorpan commented Feb 18, 2014

Although the media attribute already appears to support escapes, I'm not comfortable with doing that for srcset and sizes.

Also I think this issue is bigger than whether CSS escapes are supported. The error handling is also different from old srcset. If the value doesn't match the grammar, the whole thing is ignored. I think this makes it hard to extend the attributes in the future while providing fallback, for instance if type() is added then legacy UAs would ignore the whole attribute.

@odinho
Copy link

odinho commented Feb 19, 2014

Old srcset had it as an explicit design feature that it should be possible to extend in future.

Important catch. Question is, does anyone actually think different? How to best fix it? Take parse algorithm from original srcset?

@yoavweiss
Copy link
Member

I don't think we need to support CSS escapes in sizes and srcset.

@zcorpan
Copy link
Author

zcorpan commented Feb 20, 2014

@velmont yeah I think we should write out the parsing algorithm similar to original srcset.

@zcorpan
Copy link
Author

zcorpan commented Feb 21, 2014

Let's discuss sizes separately...

yoavweiss added a commit that referenced this issue Feb 21, 2014
Parse srcset attribute more like original srcset. Fixes #97
@yoavweiss
Copy link
Member

Closed by zcorpan@8b9af32

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants