Skip to content

Commit

Permalink
Normative: Fix absolute value bug in duration rounding no-op conditions
Browse files Browse the repository at this point in the history
Fix for bug in User Code Calls Part 2 (51ea969), spotted by Anba. Thanks!
The existing code forgot to take negative durations into account.

Closes: #2679
  • Loading branch information
ptomato committed Oct 4, 2023
1 parent 6eec81d commit 26369ba
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 4 deletions.
9 changes: 7 additions & 2 deletions polyfill/lib/duration.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
SetSlot
} from './slots.mjs';

const MathAbs = Math.abs;
const ObjectCreate = Object.create;

export class Duration {
Expand Down Expand Up @@ -288,8 +289,12 @@ export class Duration {
const balancingRequested = largestUnit !== existingLargestUnit;
const calendarUnitsPresent = years !== 0 || months !== 0 || weeks !== 0;
const timeUnitsOverflowWillOccur =
minutes >= 60 || seconds >= 60 || milliseconds >= 1000 || microseconds >= 1000 || nanoseconds >= 1000;
const hoursToDaysConversionMayOccur = (days !== 0 && zonedRelativeTo) || hours >= 24;
MathAbs(minutes) >= 60 ||
MathAbs(seconds) >= 60 ||
MathAbs(milliseconds) >= 1000 ||
MathAbs(microseconds) >= 1000 ||
MathAbs(nanoseconds) >= 1000;
const hoursToDaysConversionMayOccur = (days !== 0 && zonedRelativeTo) || MathAbs(hours) >= 24;
if (
roundingGranularityIsNoop &&
!balancingRequested &&
Expand Down
4 changes: 2 additions & 2 deletions spec/duration.html
Original file line number Diff line number Diff line change
Expand Up @@ -453,10 +453,10 @@ <h1>Temporal.Duration.prototype.round ( _roundTo_ )</h1>
1. If _maximum_ is not *undefined*, perform ? ValidateTemporalRoundingIncrement(_roundingIncrement_, _maximum_, *false*).
1. Let _hoursToDaysConversionMayOccur_ be *false*.
1. If _duration_.[[Days]] &ne; 0 and _zonedRelativeTo_ is not *undefined*, set _hoursToDaysConversionMayOccur_ to *true*.
1. Else if _duration_.[[Hours]] &ge; 24, set _hoursToDaysConversionMayOccur_ to *true*.
1. Else if abs(_duration_.[[Hours]]) &ge; 24, set _hoursToDaysConversionMayOccur_ to *true*.
1. If _smallestUnit_ is *"nanosecond"* and _roundingIncrement_ = 1, let _roundingGranularityIsNoop_ be *true*; else let _roundingGranularityIsNoop_ be *false*.
1. If _duration_.[[Years]] = 0 and _duration_.[[Months]] = 0 and _duration_.[[Weeks]] = 0, let _calendarUnitsPresent_ be *false*; else let _calendarUnitsPresent_ be *true*.
1. If _roundingGranularityIsNoop_ is *true*, and _largestUnit_ is _existingLargestUnit_, and _calendarUnitsPresent_ is *false*, and _hoursToDaysConversionMayOccur_ is *false*, and _duration_.[[Minutes]] &lt; 60, and _duration_.[[Seconds]] &lt; 60, and _duration_.[[Milliseconds]] &lt; 1000, and _duration_.[[Microseconds]] &lt; 1000, and _duration_.[[Nanoseconds]] &lt; 1000, then
1. If _roundingGranularityIsNoop_ is *true*, and _largestUnit_ is _existingLargestUnit_, and _calendarUnitsPresent_ is *false*, and _hoursToDaysConversionMayOccur_ is *false*, and abs(_duration_.[[Minutes]]) &lt; 60, and abs(_duration_.[[Seconds]]) &lt; 60, and abs(_duration_.[[Milliseconds]]) &lt; 1000, and abs(_duration_.[[Microseconds]]) &lt; 1000, and abs(_duration_.[[Nanoseconds]]) &lt; 1000, then
1. NOTE: The above conditions mean that the operation will have no effect: the smallest unit and rounding increment will leave the total duration unchanged, and it can be determined without calling a calendar or time zone method that no balancing will take place.
1. Return ! CreateTemporalDuration(_duration_.[[Years]], _duration_.[[Months]], _duration_.[[Weeks]], _duration_.[[Days]], _duration_.[[Hours]], _duration_.[[Minutes]], _duration_.[[Seconds]], _duration_.[[Milliseconds]], _duration_.[[Microseconds]], _duration_.[[Nanoseconds]]).
1. Let _precalculatedPlainDateTime_ be *undefined*.
Expand Down

0 comments on commit 26369ba

Please sign in to comment.