Last Comment Bug 720511 - arr.join("") doesn't always let indexed properties on the prototype show through
: arr.join("") doesn't always let indexed properties on the prototype show through
: addon-compat
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla11
Assigned To: Jeff Walden [:Waldo] (remove +bmo to email)
: Jason Orendorff [:jorendorff]
Depends on:
Blocks: 720079
  Show dependency treegraph
Reported: 2012-01-23 14:17 PST by Jeff Walden [:Waldo] (remove +bmo to email)
Modified: 2012-04-10 21:32 PDT (History)
4 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch with tests (4.58 KB, patch)
2012-01-23 14:17 PST, Jeff Walden [:Waldo] (remove +bmo to email)
luke: review+
Details | Diff | Splinter Review

Description Jeff Walden [:Waldo] (remove +bmo to email) 2012-01-23 14:17:05 PST
Created attachment 590870 [details] [diff] [review]
Patch with tests

Nitpicky spec compliance issue, noticed in passing while reading other code.
Comment 1 Luke Wagner [:luke] 2012-01-23 14:33:18 PST
Comment on attachment 590870 [details] [diff] [review]
Patch with tests

>+        const Value *iter;

'iter' is often the name for the iterator object so I was a bit confused when I first looked at the code.  How about 'elem'?

>+            if (iter->isObject())
>+                break;

Could you prefix this with a short comment about why the test is necessary?
Comment 2 Jeff Walden [:Waldo] (remove +bmo to email) 2012-01-23 16:25:06 PST
Comment 3 Marco Bonardo [::mak] 2012-01-24 05:03:48 PST
Comment 4 Jeff Walden [:Waldo] (remove +bmo to email) 2012-01-25 16:21:18 PST
Okay, this is kind of ridiculous, but I did some searching, and I actually found a site which breaks because of join("") not exposing prototyped properties.  (I know, right?)  I talked to akeybl about it, and he was okay taking this on aurora, so I landed it there:

The things the web does, srsly...
Comment 5 Jean-Yves Perrier [:teoli] 2012-01-26 00:54:31 PST
Aurora is Fx11 right now? Shouldn't we change the target date?
Comment 6 Jeff Walden [:Waldo] (remove +bmo to email) 2012-01-27 11:58:40 PST
Doesn't hurt to change it, no, I guess.  Version numbering in bugs is a bit fuzzy, and arguably setting the version and all that, given that we have multiple versions in flight at once, is mostly an attractive nuisance.

Notwithstanding comment 4, I doubt we need to document this.  And I doubt that even if we did document it, the people who would be affected by this bug would read our documentation such that they'd benefit from an improvement in this regard.

Note You need to log in before you can comment on or make changes to this bug.