[geometry] DOMRect: use of unrestricted doubles - min(x, NaN) #222

zcorpan · 2017-09-06T12:50:11Z

https://drafts.fxtf.org/geometry/#DOMQuad

Let left be the minimum of point 1’s x coordinate, point 2’s x coordinate, point 3’s x coordinate and point 4’s x coordinate.

See http://www.w3.org/mid/CAGN7qDCS4U=aa2bVec1OCUVSMECAgUDwZT5fSj8HS1Ue6AJozw@mail.gmail.com from @smfr
cc @cabanier

zcorpan · 2017-09-06T12:51:54Z

The email was about DOMRect but it looks like it applies for DOMQuad also.

The top attribute, on getting, must return min(y coordinate, y coordinate + height dimension).

dirkschulze · 2018-04-15T15:12:19Z

The entire conversation:

On Mon, Oct 17, 2016 at 11:21 PM, Simon Fraser smfr@me.com wrote:

On Oct 17, 2016, at 7:03 am, Rik Cabanier cabanier@gmail.com wrote:

On Sun, Oct 16, 2016 at 11:15 PM, Rik Cabanier cabanier@gmail.com wrote:

On Sun, Oct 16, 2016 at 9:21 PM, Simon Fraser smfr@me.com wrote:

On Oct 16, 2016, at 9:34 AM, Rik Cabanier cabanier@gmail.com wrote:

On Sat, Oct 15, 2016 at 2:47 PM, Simon Fraser smfr@me.com wrote:

https://drafts.fxtf.org/geometry/#DOMRect

Using unrestricted doubles forces implementors to handle NaN and Inf
values for x, y, width and height, and to correctly propagate NaN and Inf
through steps used to calculate left, top, right, bottom (assuming IEEE
rules, though the spec does not make this explicit). This is non-trivial,
since std::min<> and std::max<> do not follow the same NaN propagation
rules as Math.min()/Math.max(). Implementors have to add isnan() checks to
left, top, right, bottom implementations. Is the complexity worth it?

yes. We allowed this so NaN and Inf would signal when matrix
calculations hit edge conditions.
Instead of throwing or giving inaccurate result, it was decided to allow
the values so authors can check for those. The alternative would be to
throw and we feared that this would break a lot of code since people don't
test with exceptions.

See also the thread here: https://lists.mozilla.or
g/pipermail/dev-platform/2014-June/005091.html

When I did the implementation in Firefox, this actually made the code
easier to implement since I didn't have to add a bunch of conditionals and
could just rely on the FPU to do the correct thing.

Well, Firefox does the wrong thing for DOMRect.left() when, for example.
width is NaN (if you assume the spec follows JS Math rules).

roc implemented DOMRect and it seems that it was a simple rename of
something that Firefox already had [1]. It likely doesn't follow the spec
to the letter.

There's a test in the patch in https://bugs.webkit.org/sho

w_bug.cgi?id=163464 (which needs to be converted to a web platform
test).

Why do you need to add the special case handling? If width or x is NaN,
shouldn't left always returns x?

Sorry, I meant to say NaN
min(x, x+NaN) = min(x, NaN) = (x<NaN?x:NaN) = NaN

1: https://bugzilla.mozilla.org/show_bug.cgi?id=916520
https://bugzilla.mozilla.org/show_bug.cgi?id=916520

Correct implementation of left, top, right and bottom require two NaN
checks per call, which is an unfortunate side-effect of allowing NaN
through the interface:

https://trac.webkit.org/browser/trunk/Source/WebCore/
dom/DOMRectReadOnly.h?rev=207438#L47
https://trac.webkit.org/browser/trunk/Source/WTF/wtf/
MathExtras.h?rev=207438#L403

I see. You're interpreting min/max as math.min/max which don't follow
normal floating point rules. [1]
We should make the spec more clear on what these operators mean since we
didn't intend for this subtlety to happen.

I'd vote to put in pseudo code for min/max so you can get rid of the
special case handling on the c++ side

1: https://tc39.github.io/ecma262/#sec-math.min

@smfr what did you end up implementing? The Math.max/.min way or the C++ way? CC @bzbarsky

zcorpan · 2018-04-17T12:06:31Z

The Math.max/.min way according to https://bug-163464-attachments.webkit.org/attachment.cgi?id=291752

Demo: http://software.hixie.ch/utilities/js/live-dom-viewer/saved/5914

<!DOCTYPE html>
<script>
var q = new DOMQuad({x:NaN}, {x:NaN}, {x:NaN}, {x:NaN});
w(q.getBounds().left)
 </script>

logs NaN in Safari TP and Chrome canary; also in Firefox Nightly if I change getBounds() to bounds

zcorpan · 2018-04-17T12:14:18Z

(Filed https://bugzilla.mozilla.org/show_bug.cgi?id=1454622 about getBounds() vs bounds.)

bzbarsky · 2018-04-17T20:58:09Z

So I think the relevant testcase here is sort of like this:

<script>
  var r = new DOMRect();
  r.x = 5;
  r.width = NaN;
  alert(r.left);
</script>

here Firefox nightly and Chrome dev alert 5, while Safari Tech Preview alerts NaN.

It does seem to me that reliably returning NaN here would be somewhat better, but I'm not an expert on this code or how people use these APIs. If they use them.

Fixes w3c#222.

Currently in some geometry classes we are using std::min() and std::max() over numbers specified in IDL as "unrestricted double", meaning they could take the special value NaN. These STL helpers internally use the < operator, as in std::min(a, b) = a < b ? a : b. However, the IEEE 794 < operator always returns false when _either_ operand is NaN, so the result of min(0, NaN) and min(NaN, 0) could, confusingly, be different. This is difference is in fact visible through JavaScript. For instance, new DOMQuad({ x: NaN }, { x: 0 }, { x: 0 }, { x: 0 }).getBounds().x gives NaN, but new DOMQuad({ x: 0 }, { x: 0 }, { x: 0 }, { x: NaN }).getBounds().x gives 0. A similar problem is present for DOMRect/DOMRectReadOnly as well. This CL implements the rough consensus in [1][2], which is to adopt semantics similar to JavaScript's Math.min() and Math.max(), which propagates NaN from either operand. This also aligns our behavior with WebKit. [1]: w3c/fxtf-drafts#222 [2]: w3c/fxtf-drafts#395 Fixed: 1066499 Change-Id: Id9a4282fa00dafcfe9c5616643efbe2eaace411e

zcorpan added the geometry-1 label Sep 6, 2017

TimothyGu mentioned this issue Mar 29, 2020

SVG improvements jsdom/jsdom#2926

Open

11 tasks

TimothyGu added a commit to TimothyGu/fxtf-drafts that referenced this issue Mar 29, 2020

[geometry-1] Define minimum and maximum as preferring NaN

b9edab7

Fixes w3c#222.

TimothyGu mentioned this issue Mar 29, 2020

[geometry-1] Define minimum and maximum as preferring NaN #395

Merged

chrishtr closed this as completed in #395 Apr 1, 2020

towerofnix mentioned this issue Mar 9, 2024

[geometry-1] Is a specific handling for Infinity implied by "NaN-safe minimum/maximum"? #556

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[geometry] DOMRect: use of unrestricted doubles - min(x, NaN) #222

[geometry] DOMRect: use of unrestricted doubles - min(x, NaN) #222

zcorpan commented Sep 6, 2017

zcorpan commented Sep 6, 2017

dirkschulze commented Apr 15, 2018

zcorpan commented Apr 17, 2018

zcorpan commented Apr 17, 2018

bzbarsky commented Apr 17, 2018

[geometry] DOMRect: use of unrestricted doubles - min(x, NaN) #222

[geometry] DOMRect: use of unrestricted doubles - min(x, NaN) #222

Comments

zcorpan commented Sep 6, 2017

zcorpan commented Sep 6, 2017

dirkschulze commented Apr 15, 2018

zcorpan commented Apr 17, 2018

zcorpan commented Apr 17, 2018

bzbarsky commented Apr 17, 2018