Bug 3999

Summary:

Object.prototype is missing propertyIsEnumerable

Product:

WebKit

Reporter:

Geoffrey Garen <ggaren>

Component:

JavaScriptCore

Assignee:

Anders Carlsson <andersca>

Status:

RESOLVED FIXED

Severity:

Normal

Priority:

Version:

412

Hardware:

Mac

OS:

OS X 10.4

Attachments:

Description	Flags
Implement propertyIsEnumerable	ggaren: review-
Address comments	ggaren: review+

Geoffrey Garen

Reported 2005-07-14 10:41:02 PDT

Section 15.2.4 of ECMA-262

Attachments
Implement propertyIsEnumerable (9.30 KB, patch) 2005-12-04 13:32 PST, Anders Carlsson	ggaren: review-	Details Formatted Diff Diff
Address comments (10.78 KB, patch) 2005-12-04 14:54 PST, Anders Carlsson	ggaren: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Anders Carlsson

Comment 1 2005-12-04 13:32:08 PST

Created attachment 4942 [details] Implement propertyIsEnumerable

Geoffrey Garen

Comment 2 2005-12-04 13:42:06 PST

Comment on attachment 4942 [details] Implement propertyIsEnumerable isPropertyEnumerable should just call ObjectImp::get instead of doing the lookup manually. The putDirect calls need spaces after the commas, per our style guidelines. r- because of the above. I think it would be clearer if you named the C++ function propertyIsEnumerable instead of isPropertyEnumerable, to match the name of the corresponding (admittedly poorly named) JS function. I'll leave that decision up to you.

Anders Carlsson

Comment 3 2005-12-04 13:47:50 PST

(In reply to comment #2) > (From update of attachment 4942 [details] [edit]) > isPropertyEnumerable should just call ObjectImp::get instead of doing the > lookup manually. > The problem is that ObjectImp::get doesn't return the property flags. ObjectImp::canPut does the same thing in order to get the flags. > The putDirect calls need spaces after the commas, per our style guidelines. OK. I was just following the style that was in the file. > > r- because of the above. > > I think it would be clearer if you named the C++ function propertyIsEnumerable > instead of isPropertyEnumerable, to match the name of the corresponding > (admittedly poorly named) JS function. I'll leave that decision up to you. > Sounds like a good idea.

Anders Carlsson

Comment 4 2005-12-04 14:54:53 PST

Created attachment 4949 [details] Address comments

Geoffrey Garen

Comment 5 2005-12-04 15:18:48 PST

Comment on attachment 4949 [details] Address comments ggaren: andersca: [3:10pm] ggaren: + return !(attributes & DontEnum); [3:10pm] ggaren: is backwards, no? [3:12pm] ggaren: + putDirect(valueOfPropertyName, new ObjectProtoFuncImp(exec, funcProto, ObjectProtoFuncImp::ValueOf, 0), DontEnum); [3:12pm] ggaren: has an extra space Otherwise, looks great.

Geoffrey Garen

Comment 6 2005-12-04 15:26:35 PST

Comment on attachment 4949 [details] Address comments My bad. Not Don't Enum == enumberable. r=me

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