Bug 9903

Summary: Simplify logic in JSHTMLOptionsCollection::setLength() by using no-arg getNumber()
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: JavaScriptCoreAssignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, ggaren, mrowe
Priority: P2    
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
Patch v1
none
Patch v2 ggaren: review+

David Kilzer (:ddkilzer)
Reported 2006-07-13 20:29:17 PDT
This is a follow-up to Bug 9742.  See Bug 9742 Comment #15 for Geoffrey's comments about why this should be fixed. Adding NeedsRadar keyword since it's a follow-up to bug 9742, which also had NeedsRadar.
Attachments
Patch v1 (1.54 KB, patch)
2006-07-13 21:21 PDT, David Kilzer (:ddkilzer) 		no flags
DetailsFormatted DiffDiff
Patch v2 (1.70 KB, patch)
2006-07-13 22:33 PDT, David Kilzer (:ddkilzer) 		ggaren: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
David Kilzer (:ddkilzer)
Comment 1 2006-07-13 21:21:34 PDT
Created attachment 9438 [details] Patch v1 Proposed patch.  (I hope I interpreted your comment correctly, Geoff!)
David Kilzer (:ddkilzer)
Comment 2 2006-07-13 21:58:34 PDT
Comment on attachment 9438 [details] Patch v1 Oops. I need to include the bug information in the ChangeLog!
David Kilzer (:ddkilzer)
Comment 3 2006-07-13 22:33:31 PDT
Created attachment 9439 [details] Patch v2 Now with Bugzilla bug information in the ChangeLog!
David Kilzer (:ddkilzer)
Comment 4 2006-07-13 22:45:51 PDT
Note that the tests added in Bug 9742 still pass with Patch v2.
Geoffrey Garen
Comment 5 2006-07-14 01:27:09 PDT
Comment on attachment 9439 [details] Patch v2 That's right. This patch enables landing the getNumber part of my JavaScriptCore patch: { if (JSImmediate::isImmediate(this)) { v = JSImmediate::toDouble(this); - return true; + return JSImmediate::isNumber(this); } return downcast()->getNumber(v);
David Kilzer (:ddkilzer)
Comment 6 2006-07-14 05:00:19 PDT
Committed revision 15424.
David Kilzer (:ddkilzer)
Comment 7 2007-06-24 12:04:43 PDT
Mass removal of NeedsRadar keyword from my bugs that have already been RESOLVED.
Note You need to log in before you can comment on or make changes to this bug.