Bug 34213 - Add layout tests to verify assignment to items of NodeList
Summary: Add layout tests to verify assignment to items of NodeList
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-27 06:07 PST by anton muhin
Modified: 2010-01-28 09:04 PST (History)
2 users (show)

See Also:


Attachments
Patch (2.38 KB, patch)
2010-01-27 06:17 PST, anton muhin
no flags Details | Formatted Diff | Diff
First take (2.41 KB, patch)
2010-01-27 07:04 PST, anton muhin
no flags Details | Formatted Diff | Diff
Reference to Chromium bug added (2.58 KB, patch)
2010-01-28 05:26 PST, anton muhin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description anton muhin 2010-01-27 06:07:43 PST
Add layout tests to verify assignment to items of NodeList
Comment 1 anton muhin 2010-01-27 06:17:15 PST
Created attachment 47526 [details]
Patch
Comment 2 anton muhin 2010-01-27 07:04:34 PST
Created attachment 47528 [details]
First take
Comment 3 anton muhin 2010-01-27 07:05:17 PST
(In reply to comment #2)
> Created an attachment (id=47528) [details]
> First take

It passes on both WebKit and Chromium's ToTs.
Comment 4 Alexey Proskuryakov 2010-01-27 13:49:35 PST
Is this a regression test for some previously fixed bug? Why 13 iterations?
Comment 5 anton muhin 2010-01-27 14:05:52 PST
(In reply to comment #4)
> Is this a regression test for some previously fixed bug? Why 13 iterations?

Yes, precisely: http://code.google.com/p/chromium/issues/detail?id=27967

Technically for current V8 2 is enough, but I thought that 13 is a reasonable compromise.  If you prefer to see smaller constant, np, I'd only ask it to be >= 2, ideally >= 3
Comment 6 Alexey Proskuryakov 2010-01-27 14:15:16 PST
Comment on attachment 47528 [details]
First take

r=me

I like to have a reference to the bug fixed in test output, but that's not really important, one can always do svn log.
Comment 7 anton muhin 2010-01-28 05:26:28 PST
Created attachment 47607 [details]
Reference to Chromium bug added
Comment 8 anton muhin 2010-01-28 05:27:27 PST
(In reply to comment #6)
> (From update of attachment 47528 [details])
> r=me
> 
> I like to have a reference to the bug fixed in test output, but that's not
> really important, one can always do svn log.

I've added it into description.  Is it fine?
Comment 9 Alexey Proskuryakov 2010-01-28 08:23:38 PST
Comment on attachment 47607 [details]
Reference to Chromium bug added

It is a little strange to see a link on a Web page that's plain text, and not clickable (one can use HTML tags in JS test description). But it's not a practical issue, no need to further change the patch.
Comment 10 anton muhin 2010-01-28 08:25:57 PST
Comment on attachment 47607 [details]
Reference to Chromium bug added

Thanks a lot for review, Alexey.
Comment 11 WebKit Commit Bot 2010-01-28 09:03:56 PST
Comment on attachment 47607 [details]
Reference to Chromium bug added

Clearing flags on attachment: 47607

Committed r54004: <http://trac.webkit.org/changeset/54004>
Comment 12 WebKit Commit Bot 2010-01-28 09:04:03 PST
All reviewed patches have been landed.  Closing bug.