Fix utf-8 support for dataUri base64 #14
Conversation
xiaoxiangmoe
force-pushed
the
master
branch
3 times, most recently
from
1c871c5 to
d63937e
Compare
lib/source-map-resolve-node.js
Outdated
| .map(function(x) { | ||
| return x.slice('charset='.length) | ||
| }) | ||
| .pop() || defaultCharset |
There was a problem hiding this comment.
Do we know what happens if charset is provided multiple times? Is it an error? Does the first one win? The last one?
I also think we should do something like this:
var dataUriRegex = /^data:([^,;]*)(?:;([^,;]+))?(?:,(.*))?$/
var mimeType = dataUri[1]
var parameters = (dataUri[2] || "").split(";")
var encoded = dataUri[3] || ""
var charsetRegex = /^charset=(.+)$/
var charset = parameters.reduce(function(result, param) {
var match = charsetRegex.exec(param)
return match ? match[1] : result
}, "us-ascii")
// ...
parameters[parameters.length - 1] === "base64" ?There was a problem hiding this comment.
first one win in Firefox and chrome.
and more， https://tools.ietf.org/html/rfc8259#section-8.1 only allow use utf-8. maybe we could ignore charset and always use utf-8
There was a problem hiding this comment.
Always using utf-8 sounds good to me!
There was a problem hiding this comment.
should we check charset and console.warn or throw new Error if charset is not utf-8？
There was a problem hiding this comment.
I don’t think that’s necessary. I think we can just assume utf-8.
There was a problem hiding this comment.
@lydell Hi，Do you have time to add a test?
There was a problem hiding this comment.
Hi, thanks for the reminder! I’m working on it now.
There was a problem hiding this comment.
@lydell you can push to my repo now.
There was a problem hiding this comment.
Thanks, but I’m too lazy to move back over here now.
xiaoxiangmoe
changed the title
close #13
@lydell