-
-
Notifications
You must be signed in to change notification settings - Fork 893
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
Fixes for non-compliant Date
, multiple browsers.
#360
Conversation
@@ -1007,6 +1007,44 @@ defineProperties($Object, { | |||
// ==== | |||
// | |||
|
|||
var hasNegtiveMonthYearBug = new Date(-3509827334573292).getUTCMonth() === 12; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/Negtive/Negative
On Opera 11.6, this passes the |
I'll take a further look at this as soon as I get a bit of spare time. I don't have access to Opera 10.6 but I do the other two. My access to any Opera environments is very limited, at times non-existent. I am still trying to get back to looking at this. I was able to test Opera 11.6 and it passed, all green, but that's all I have access to at the moment. I managed to test Opera 12 and that fails the Been trying to get on Managed to get on Opera 10.6 and it crashed when I ran tests. :/ This patch now fixes No matter what I've tried, I am unable to get IE11 and 20 to take the Ok, I think I've figured out what is going on with IE11 and 20 now, it concerns their representation of negative dates. That now fixes everything that I can test, except as mentioned the |
b4560ee
to
0471bd9
Compare
Any ideas about discovering the problem with Opera 10.6? |
None, sadly. All I can think of is commenting parts out until it doesn't crash, and narrowing down which line does it. |
I've downloaded and installed Opera 10.6 on my systems and started doing just that. I commented out all the tests, it crashed. I commented out all the code, it ran. I started putting code back in and this line (may not be the only one) caused it to crash when reinstated.
I changed the negative date to
and uncommented the line and it now runs, will try reinstating the rest of the code. But it looks like it doesn't like a date that far in the past with Haven't tried catch or anything yet, but
But I can't find a core dump file. Nope, try...catch does nothing, it just bums out saying core dump. I'll see if I can use the original test dates for everything except the ones using Opera 10.6 now runs without crashing. I added some canned data for months, and I feel it could really use some more for days of the month: just to be 100% sure that all is working, but I don't have time just now. I haven't performed any browser tests on this rebase, other than Opera 10.6 |
What was the change that prevented/caused the crash? |
getMonth
, getFullYear
, getUTCMonth
, getUTCFullYear
for negative years.Date
, multiple browsers.
I changed the test date for these detections
And once detected and patched then they no longer cause a problem with crashing in the tests either. Just the natives broken. |
Added tests for first and last day of each month, Opera 10.6 passing, not tested other browsers. I was just running a test or two, Opera 11.64 on Window 7 fails 3 tests Added a fix for the weirdness and now passes Opera 11.64 on Windows and Linux. Tested a few other browsers, IE8,9,10,11 & 20 One thing that could be improved is the bug detection/tests when the timezone is +1200 to +1400, that's all those little islands along the International Date Line That should fix it for the timezone differences with bug detection, tested on my local browsers only, where I have control over the timezone offset. Still need to check the tests. Opera 11.64 on windows now appears broken again I seems that the Saucelabs Opera 11.64 is either not emptying its cache or there is a proxy that is serving cached files. This appears to be the reason for the difference, as when I look though the source it is not getting my recent changes. Doh! I'm an idiot and was pasting the wrong URL! |
70d7337
to
4a4f88e
Compare
I believe that this patch is ready to go, unless you can find any issues with it? |
All set! There's still some 10.6 issues, but not new ones. I've left those in #240. |
#240
Add extra tests to check string and other values.
This highlights differences between environments, i.e. strings. It depends on how far you want to take fixes/normalisation?
A version of
eslint
errors with theses changes, but not on any code that has been added.