Bug 12807

Summary: XPath incorrectly converts NaN to boolean
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: XMLAssignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal CC: darin
Priority: P2    
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
Bug Depends on:    
Bug Blocks: 12584    
Attachments:
Description Flags
proposed fix adele: review+

Alexey Proskuryakov
Reported 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.
Attachments
proposed fix (3.79 KB, patch)
2007-02-18 09:06 PST, Alexey Proskuryakov
adele: review+
Alexey Proskuryakov
Comment 1 2007-02-18 09:06:50 PST
Created attachment 13227 [details] proposed fix Fixes that, and also the substring-after() function.
Alexey Proskuryakov
Comment 2 2007-02-18 13:19:13 PST
Committed revision 19697.
Darin Adler
Comment 3 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
Alexey Proskuryakov
Comment 4 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.
Alexey Proskuryakov
Comment 5 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!
Alexey Proskuryakov
Comment 6 2007-02-20 00:55:20 PST
Wrong again - I actually believe the landed fix is correct :-)
Darin Adler
Comment 7 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.
Note You need to log in before you can comment on or make changes to this bug.