Comments (31)
@thomascarlsen86
Thank you for filing the bug. From what I know, the bug is caused by Firefox
not treating the regex global flag the same as the rest of the browsers do.
More info can be found on the issue in the following link:
http://stackoverflow.com/q/4178970/290340
As soon as I come up with a fix I'll incorporate it into the next release.
Original comment by [email protected]
on 2 Jul 2012 at 12:49
- Added labels: Priority-Critical
- Removed labels: Priority-Medium
from jquery-csv.
[deleted comment]
from jquery-csv.
thanks for the quick reply, and sad to hear that this is't a quick fix :(
Original comment by [email protected]
on 2 Jul 2012 at 12:55
from jquery-csv.
Man, I totally made this code work last night perfectly and beautifully in
Chrome, and then this morning opened it up in Firefox and ran into exactly this
issue. Noooooo!
http://blog.thatscaptaintoyou.com/strange-behavior-of-the-global-regex-flag/
might have help you. It does say "The solution to this dilemma is painfully
simple, if the regex is global just set the “lastIndex” property to zero
before you test.". I would do it myself, but I haven't dug into your code deep
enough to know where to fiddle.
Original comment by [email protected]
on 4 Jul 2012 at 8:30
from jquery-csv.
The problem is showing up for me with this CSV file. It seems that adding
quotes (as in the first two entries there) makes firefox behave. Strange that
the other browsers work fine even on the non-quoted ones, though meta.delimiter
is defaulting to """.
Original comment by [email protected]
on 4 Jul 2012 at 8:42
Attachments:
from jquery-csv.
@kousue The code is actually very simple, csv2Array and csv2Dictionary handle
the split by line then each line is passed to csvEntry2Array which handle the
more complex CSV parsing. csvEntry2Array is where you'll find the regexes that
need to be modified.
If you want to submit a patch, I'll get it incorporated ASAP. Otherwise, life
has me busy and it'll be a couple of weeks before I can get a fix working,
tested, and released.
If you decide to clone the source you'll also find a working test runner with
directions. I'll drop a minor release (0.61) now that has the latest.
As for the delimiter defaulting to a double quote, that's should be correct
according to the IETF RFC for CSV files. If you use something else (ex
backslash) it can be changed in the function call parameters.
I'm not sure why quoting all values works in Firefox, maybe that'll work as a
temporary solution until the bug is resolved.
Original comment by [email protected]
on 4 Jul 2012 at 10:06
from jquery-csv.
BTW, the parameter to change the escape char is called 'escaper' and will be
available in release 0.61.
Original comment by [email protected]
on 4 Jul 2012 at 10:58
from jquery-csv.
Original comment by [email protected]
on 4 Jul 2012 at 10:58
- Changed state: Accepted
from jquery-csv.
I spent some time messing with the code. Everywhere where a global flag could
be a problem has been resolved. I couldn't make it more global flag proof if I
tried.
After staring cross-eyed at the test results for a while, it appears that only
the fields that aren't delimited don't get matched (probably could have saved
some time by reading the comments more closely). The even stranger part is, the
data IS passing the validation step (else it would return null for non-matches)
it's just the str.replace matching that is broken.
I'm going to try implementing the matching using the more robust RegExp.exec
method next. Until this issue is fixed, you'll need to have your CSV data
delimited for the parser to work in Firefox.
If you want to help, clone the source and take a look at the code or run the
test runner in some other browsers (ie, Safari, IE, Opera, etc) and post your
results.
Original comment by [email protected]
on 5 Jul 2012 at 1:28
- Changed state: Started
from jquery-csv.
[deleted comment]
from jquery-csv.
I have done this for a quick hack:
$.csvEntry2Array = function(csv, meta) {
// csv hack for FF
csv = '"'+csv.replace(/,/g,'","')+'"';
.......
Original comment by [email protected]
on 9 Jul 2012 at 12:15
from jquery-csv.
Any chance this will work with mixed delimited/non-delimited values in FF?
Unfortunately I don't have the luxury of changing the source CSV I'm working
with (it gets automatically updated).
Original comment by [email protected]
on 20 Jul 2012 at 6:26
from jquery-csv.
My quick hack will work with a mixed csv, but I still don't think its a pretty
solution, but it will work out for you ;)
Original comment by [email protected]
on 20 Jul 2012 at 6:29
from jquery-csv.
Actually nope, it doesn't work because the delimited fields have commas inside
of them:
id, "delimited, field, here", other field, another field, "one more delimited
field", etc
Original comment by [email protected]
on 20 Jul 2012 at 6:34
from jquery-csv.
okay, this ain't prettier but you can do this:
$.csvEntry2Array = function(csv, meta) {
// csv hack for FF
csv = "'"+csv.replace(/"/g,"'").replace(/,/g,"','")+"'";
and then set your "delimiter" to delimiter:"'", at the top og this file
Original comment by [email protected]
on 20 Jul 2012 at 6:41
from jquery-csv.
That doesn't work, probably related to the random ' characters in the CSV.
Original comment by [email protected]
on 20 Jul 2012 at 6:46
from jquery-csv.
I have to say that you have a realy f**ked up csv file m8. :)
Original comment by [email protected]
on 20 Jul 2012 at 6:49
from jquery-csv.
Any progress on this? I tried the above suggestions, but I don't think I did it
right. They didn't help. I don't have, right now, a funky csv file.
Original comment by [email protected]
on 13 Aug 2012 at 9:50
from jquery-csv.
@Chad.R.Bernier None yet, my personal life has me occupied. I'll try to take
another shot at it over the next few days.
Original comment by [email protected]
on 16 Aug 2012 at 3:45
from jquery-csv.
[deleted comment]
from jquery-csv.
In the callback function for String.replace, it appears that whenever a match
is not found for a parenthesized submatch string, Firefox (and IE <= 8)
represents the result as an empty string, whereas Chrome (and IE 9+) uses
undefined. Run this quick test in your browser's console to confirm this
behavior (the regular expression rex uses the relevant part of reValue,
assuming a separator and delimiter of "):
var rex = /"([^""]*(?:""[^""]*)*)"|([^,""\s]*(?:\s+[^,""\s]+)*)/,
test1 = '"Hello!"',
test2 = 'Hello!';
function replaceCallback(m0, m1, m2) {
console.log('matched substring (m0): ' + m0 + '\n'
+ 'submatch 1 (m1): \n'
+ '\t type: ' + (typeof m1) + '\n'
+ '\t as string: ' + m1 + '\n'
+ 'submatch 2 (m2): \n'
+ '\t type: ' + (typeof m2) + '\n'
+ '\t as string: ' + m2 + '\n');
return '';
}
test1.replace(rex, replaceCallback); // m2 is empty string in FF, undefined in Chrome
test2.replace(rex, replaceCallback); // m1 is empty string in FF, undefined in Chrome
In jQuery-CSV, this difference in behavior causes undesirable results in
Firefox, due to the conditions on lines 88 and 93 of version 0.61 which expect
a value of "undefined" for submatches that are not found. Here is revision to
those two conditions that accounts for both undefined and empty string
possibilities:
csv.replace(reValue, function(m0, m1, m2) {
// Remove backslash from any delimiters in the value
if(typeof m1 === 'string' && m1.length) { // Fix: evaluates to false for both empty strings and undefined
var reDelimiterUnescape = /ED/g;
reDelimiterUnescape = RegExp(reDelimiterUnescape.source.replace(/E/, escaper), 'g');
reDelimiterUnescape = RegExp(reDelimiterUnescape.source.replace(/D/, delimiter), 'g');
output.push(m1.replace(reDelimiterUnescape, delimiter));
} else if(typeof m2 === 'string' && m2.length) { // Fix: evaluates to false for both empty strings and undefined
output.push(m2);
}
return '';
});
With these changes in place, the plugin is working perfectly for my needs on
all tested browsers :D. I hope this addresses the Firefox issues that some of
you encountered.
Original comment by [email protected]
on 27 Aug 2012 at 10:01
from jquery-csv.
Comment #21 is probably a good lead to understanding the problem, but it breaks
Chrome, as shown by the test suite.
In particular, empty strings are perfectly legitimate values, and need to be
handled.
Original comment by [email protected]
on 4 Sep 2012 at 2:49
from jquery-csv.
Ah. It passes the test if you accept #21's first change, and not his second.
The problem boils down to -- if the input really IS the null string value, you
don't want to reject it from both clauses.
But you also want to add a:
if (csv === "") {
return [""];
}
before the validity check that returns null, as an empty string is logically
one empty value (just as a comma is logically two empty values). This appears
to be consistent with the ABNF in the RFC.
You can also get rid of 5 constructions of RegExp's along the way -- you don't
need to construct each intermediate partially-substituted RegExp. Perhaps you
did it this way to make bugs show up better? I don't know if constructing a
RegExp is expensive enough to worry about or not, in the grand scheme of
things. It does work to remove them.
I constructed a bunch more test cases, with finer granularity, using jasmine,
when I imported this into my system. Those were helpful.
The git repository does not appear to be up-to-date!
But I'm attaching patches from what was distributed as jquery.csv-0.61.js
0001 fixes some lint
0002 fixes the bugs
0003 removes the extraneous RegExps.
The patches are independent; you can apply any subset in any order.
Also, I notice that the existing test cases don't cover embedded quotes in
strings; that's a case that ought to be covered. I'll add it to my suite, and
report it as a separate issue if it fails.
Original comment by [email protected]
on 4 Sep 2012 at 9:19
from jquery-csv.
For your patching convenience -- I reconstructed in my clone of your public
repository, what should be the missing push, so I could construct patches you
should be able to apply directly.
We'll see what kind of chaos I git when I eventually pull the missing commit. :)
I also added an additional patch. This one removes the Unicode Byte-Order-Mark
(BOM) from the start of the file. This confuses some tools when jsmin goes and
sticks it in the middle of the file, which is deprecated in Unicode 3.2 (and
completely defeats any purpose it might have had!).
I made it separate in case you really have a need for a BOM -- or if your tools
will just stick it back anyway.
You might need to supply --ignore-space-change to 'git am' to apply these, but
they should apply OK on top of your 0.61 release point.
I'm removing the patches from my previous comment to avoid confusion. Note that
the numbering has changed:
0001 removes BOM
0002 fixes some lint
0003 fixes the bugs
0004 removes the extraneous RegExps.
Original comment by [email protected]
on 4 Sep 2012 at 8:59
Attachments:
- 0001-Remove-Unicode-Byte-Order-Mark-at-start-of-file-beca.patch
- 0002-Fix-up-some-lint-that-will-make-tools-and-spell-chec.patch
- 0003-Fix-Issue-5-Firefox-compatibility.patch
- 0004-Remove-extraneous-RegExp-creation.patch
from jquery-csv.
@[email protected] Thank you for your contribution. You're right that I didn't push
the last changes. I had some issues on my dev VM when Debian pulled in Gnome 3.
The release was stuck on 0.60 even though the src said 0.51. Everything is
fixed now.
Nice catch on the BOM issue. I currently don't run tests on the minified src,
I'll add that to the TODO list. Looks like I need a need to get Notepad++
running under WINE too. I really wish somebody would fork and port NPP to *nix.
Not too clear about 0004. You use 3 different types of replace on the regex
source. Is there a performance reason for doing it that way.
I think comment #14 might still need to be addressed. We'll see.
I'll post an update as soon as I have the changes pushed to the remote origin.
Original comment by [email protected]
on 5 Sep 2012 at 6:34
from jquery-csv.
@[email protected] I'm going to hold off on 0004 for now. First, because I want to
get this bugfix texted and verified first. Second, because I had something else
in mind to address the performance issues. See bug 6 for more details.
Original comment by [email protected]
on 5 Sep 2012 at 5:23
- Changed state: Fixed
from jquery-csv.
Thank you to everybody who participated in this bugfix.
If you have the time, please run your data through the parser to verify that
there aren't any other issues.
If nobody posts any other problems, I'd like to mark this bug as 'verified' and
move on.
Original comment by [email protected]
on 5 Sep 2012 at 5:52
from jquery-csv.
[I didn't hit Submit on this, so this reply is appearing out-of-sequence. Since
you're heading in another direction anyway, it's just FYI...]
Re: 0004 -- no, just one kind of replace. There are just three contexts.
In the first case, you're pulling it from a supplied RegExp:
reValue = reValue.source.replace(/X/g, separator);
In the second case, reValue is now a string, so there's no .source. We just
replace.
reValue = reValue.replace(/Y/g, delimiter);
In the third case, we again have a string, but we want a RegExp, so we re-wrap
our final string in a RegExp again:
reValue = RegExp(reValue.replace(/Z/g, escaper), 'g');
Perhaps it would be more clear as:
var reValueStr = reValue.source;
reValueStr = reValueStr.replace(/X/g, separator);
reValueStr = reValueStr.replace(/Y/g, delimiter);
reValueStr = reValueStr.replace(/Z/g, escaper);
reValue = RegExp(reValueStr, 'g');
BTW, I tested with fields with commas; they work fine with these changes.
Original comment by [email protected]
on 5 Sep 2012 at 11:35
from jquery-csv.
Gotcha, I prefer the second form.
I'll add that last change, run the tests, and if everything's good, drop 0.62
tonight.
I want to get this bug fix released asap before anybody else downloads the 0.61
source.
Nice work.
Original comment by [email protected]
on 6 Sep 2012 at 12:31
from jquery-csv.
Original comment by [email protected]
on 11 Oct 2012 at 4:07
from jquery-csv.
Original comment by [email protected]
on 15 Oct 2012 at 10:39
- Changed state: Verified
from jquery-csv.
Related Issues (20)
- doesn't seem to recoginse end of line properly in some cases. HOT 1
- CR line ending are not well treated HOT 1
- Typo in basic-usage.html (fixed in 0.8.0) HOT 2
- Enhancement: Parse CSV in a thread using settimeout HOT 3
- 배열의 기본 속성값(remove)이 포함 됩니다. HOT 1
- Karma test fails - HOT 2
- Failures on test.html file HOT 2
- colNum not reset when onParseValue callback called
- Export $ globally in browser when not already set
- CSVDataError: Illegal State - Only in IE HOT 3
- Using Zurb Foundation 3 causing an error HOT 3
- $.csv.toObjects return garbage value HOT 3
- Parse doesn't handle quotes in the CSV, even if escaped HOT 3
- Hooks relying on a return value of false to skip entries makes parsing values that resolve to false impossible
- CSV file created with Excel for Mac fails HOT 3
- Uncaught ReferenceError: require is not defined
- Generate Motion chart HOT 1
- Options arg not really optional? HOT 2
- TypeError: csv.replace is not a function
- Data Format
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from jquery-csv.