Bug 12807 - XPath incorrectly converts NaN to boolean
Summary: XPath incorrectly converts NaN to boolean
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: XML (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on:
Blocks: 12584
  Show dependency treegraph
 
Reported: 2007-02-18 08:41 PST by Alexey Proskuryakov
Modified: 2007-02-20 09:22 PST (History)
1 user (show)

See Also:


Attachments
proposed fix (3.79 KB, patch)
2007-02-18 09:06 PST, Alexey Proskuryakov
adele: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2007-02-18 08:41:51 PST
From the spec: "a number is true if and only if it is neither positive or negative zero nor NaN" - but XPath::Value::toBoolean() only checks for 0.
Comment 1 Alexey Proskuryakov 2007-02-18 09:06:50 PST
Created attachment 13227 [details]
proposed fix

Fixes that, and also the substring-after() function.
Comment 2 Alexey Proskuryakov 2007-02-18 13:19:13 PST
Committed revision 19697.
Comment 3 Darin Adler 2007-02-19 13:01:29 PST
The more elegant and efficient way to fix bugs like this is to take into account the fact that comparison expressions including NaN always return false.

So this change would have been my recommendation:

-            return m_number != 0;
+            return !(m_number == 0); // true for NaN
Comment 4 Alexey Proskuryakov 2007-02-20 00:30:43 PST
(In reply to comment #3)
> +            return !(m_number == 0); // true for NaN

I'm not sure if this idiom can be used here - we want to convert NaN to false, not to true.
Comment 5 Alexey Proskuryakov 2007-02-20 00:32:15 PST
Oh... So, I think there wasn't any bug here at all - it was the other change in this patch that fixed the bug!
Comment 6 Alexey Proskuryakov 2007-02-20 00:55:20 PST
Wrong again - I actually believe the landed fix is correct :-)
Comment 7 Darin Adler 2007-02-20 09:22:33 PST
(In reply to comment #6)
> Wrong again - I actually believe the landed fix is correct :-)

I believe that no matter which behavior we want for NaN, one of (m_number != 0.0) or !(m_number == 0.0) should work. I'm surprised to hear that's not true.