Bug 21801 - REGRESSION (r37821): YUI date formatting JavaScript puts the letter 'd' in place of the day
Summary: REGRESSION (r37821): YUI date formatting JavaScript puts the letter 'd' in pl...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P1 Normal
Assignee: Cameron Zwarich (cpst)
URL: http://developer.yahoo.com/yui/exampl...
Keywords: HasReduction, InRadar, Regression
Depends on:
Blocks:
 
Reported: 2008-10-22 09:41 PDT by Nate Silva
Modified: 2008-11-07 13:29 PST (History)
3 users (show)

See Also:


Attachments
Reduction (397 bytes, text/html)
2008-11-06 21:07 PST, Cameron Zwarich (cpst)
no flags Details
Fix (952 bytes, patch)
2008-11-07 01:41 PST, Cameron Zwarich (cpst)
no flags Details | Formatted Diff | Diff
Proposed patch (12.51 KB, patch)
2008-11-07 13:11 PST, Cameron Zwarich (cpst)
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nate Silva 2008-10-22 09:41:33 PDT
SUMMARY

When using the YUI DataTable object, a column formatted as:

  {key:'date', formatter:YAHOO.widget.DataTable.formatDate, 
                  sortable:true, resizeable:true}

...shows the date incorrectly. For example, a date which should be shown as "03/24/1980" is instead shown as "03/d/1980".

This does not happen on Safari 3, but it does happen on the latest WebKit nightly (r37764).

STEPS TO REPRODUCE

Go to the Yahoo UI example at:
http://developer.yahoo.com/yui/examples/datatable/dt_basic_clean.html
Comment 1 Nate Silva 2008-10-22 09:53:36 PDT
This has also been filed as YUI bug #2187202.
https://sourceforge.net/tracker2/?func=detail&aid=2187202&group_id=165715&atid=836476
Comment 2 Alexey Proskuryakov 2008-10-23 06:27:23 PDT
Confirmed as a regression with r37807.
Comment 3 Nate Silva 2008-11-03 16:38:50 PST
Filed with Apple's Bug Reporter (is that what "NeedsRadar" means?). The bug report is:

rdar://problem/6341231
Comment 4 Alexey Proskuryakov 2008-11-04 03:53:00 PST
Thank you! NeedsRadar isn't necessarily a request for the reporter to file it with Apple (someone else can do that), but it is slightly better if both bugs are technically reported by the same person.
Comment 5 Cameron Zwarich (cpst) 2008-11-06 18:38:46 PST
I have determined that this regressed between r36766 and r36847. I am also assigning this to myself.
Comment 6 Cameron Zwarich (cpst) 2008-11-06 18:59:38 PST
There was a typo in my previous post. This regressed in r36821:

http://trac.webkit.org/changeset/36821
Comment 7 Cameron Zwarich (cpst) 2008-11-06 21:07:37 PST
Created attachment 24962 [details]
Reduction

Here is a reduction. I can't seem to reduce it any further. The code in the script tag also works in the commandline JSC shell.
Comment 8 Cameron Zwarich (cpst) 2008-11-07 01:05:48 PST
The problem here is that the check in CodeGenerator::emitTypeOf() to determine whether a register is constant is wrong after r36821:

            && static_cast<unsigned>(src2->index()) < m_codeBlock->constantRegisters.size()

I am kind of surprised that nothing else hit this.
Comment 9 Cameron Zwarich (cpst) 2008-11-07 01:41:48 PST
Created attachment 24967 [details]
Fix

Here is the simple fix. I will make a bunch of tests based on this original pattern, covering all of the individual typeof cases.
Comment 10 Geoffrey Garen 2008-11-07 10:21:53 PST
In r36821, I had to fix many call sites making custom calculations about whether a register was constant. I guess I missed one.

I think it would be nice to fix this bug by changing this call site to use the isConstant function I wrote -- that way, the next time constant register layout changes, we won't have the same problem again.
Comment 11 Cameron Zwarich (cpst) 2008-11-07 13:11:34 PST
Created attachment 24972 [details]
Proposed patch

Here is a better fix that uses the isConstant() member function I just added to CodeBlock.
Comment 12 Geoffrey Garen 2008-11-07 13:12:58 PST
Comment on attachment 24972 [details]
Proposed patch

r=me
Comment 13 Cameron Zwarich (cpst) 2008-11-07 13:29:51 PST
Landed in r38230.