Conversation
a6d6513 to
634870c
Compare
634870c to
db8b167
Compare
|
The tests are failing here, and I still think that we need tests for the decimal changes. |
|
I spent quite a bit of time trying to make a test case for the decimal change but so far I haven’t been able to. I did uncover some discrepancies between Firefox and Chrome with regards to precision though, even they are both using a big decimal implementation. (The same test has the same results in jsdom before/after the change.) The tricky part is that the spec doesn’t seem to mandate a specific level of precision, so it’s unclear who exactly is correct. I’ll fix the test failure when I get to it. |
db8b167 to
8fde61d
Compare
|
In light of whatwg/html#5207, I'd rather not add tests for the decimal change. We already know that it "works" in the sense that it doesn't regress what used to work. Otherwise, fixed the test failures. |
|
But, why include it at all, if it doesn't help us in a testable observable way? What if we regress, with no tests to catch us? I'd rather remove that change, if we don't think it's worth testing. |
|
I'm going to pull the other two commits into master :) |
|
Oh, I guess that isn't so straightforward, since the third depends on the under-discussion second. Nevermind, for now! |
This is more consistent with the rest of jsdom. Also make sure there is no Unicode whitespace before the number appears.
stepMismatch currently has its own way of checking whether the value is aligned, even though the _isStepAligned function was written for this very purpose. Move the decimal.js-based modulo check to _isStepAligned, and make stepMismatch use that function instead. Also rename the parameter for _stepAlign so that it makes more sense.
It is not a concept in the spec, and the call sites were awkward with it anyway.
|
Okay, I've removed the controversial change to |
8fde61d to
35a002f
Compare
This is rebased on top of #2786. Creating a draft for Travis to take a look at it first.