Closed
Bug 146116
Opened 22 years ago
Closed 22 years ago
Image.prototype.foo not delegated to by (new Image).foo
Categories
(Core :: DOM: Core & HTML, defect, P3)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla1.2alpha
People
(Reporter: brendan, Assigned: peterv)
References
Details
Attachments
(2 files, 2 obsolete files)
1.76 KB,
text/html
|
Details | |
12.32 KB,
patch
|
timeless
:
review+
peterv
:
superreview+
|
Details | Diff | Splinter Review |
We need Image.prototype to refer to HTMLImageElement.prototype. /be
Reporter | ||
Updated•22 years ago
|
Keywords: 4xp,
mozilla1.1
Comment 1•22 years ago
|
||
Peterv might be taking this bug sometime soon... We should do the same for Option.prototype too.
Target Milestone: --- → mozilla1.1alpha
Assignee | ||
Comment 2•22 years ago
|
||
Taking.
Assignee: jst → peterv
Priority: -- → P3
Target Milestone: mozilla1.1alpha → mozilla1.2alpha
Assignee | ||
Updated•22 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•22 years ago
|
||
I added a new category (JAVASCRIPT_GLOBAL_CONSTRUCTOR_PROTO_CATEGORY) that allows to specify the prototype that a global constructor should share.
Comment 4•22 years ago
|
||
Comment on attachment 93405 [details] [diff] [review] v1 +#define JAVASCRIPT_GLOBAL_CONSTRUCTOR_PROTO_CATEGORY \ + "JavaScript global constructor prototype" Wouldn't it make things clearer if the word "alias" was somewhere in that macro name? + } else if (name_struct->mType == nsGlobalNameStruct::eTypeConstructorAlias) { + nsGlobalNameStruct* alias_struct = + gNameSpaceManager->GetConstructorProto(name_struct); + if (alias_struct) { + if (alias_struct->mType == nsGlobalNameStruct::eTypeClassConstructor) { ... Here you make sure GetConstructorProto doesn't return null. @@ -3519,11 +3534,15 @@ JSObject *dot_prototype = nsnull; + if (name_struct->mType == nsGlobalNameStruct::eTypeConstructorAlias) { + name_struct = gNameSpaceManager->GetConstructorProto(name_struct); + } + if (name_struct->mType == nsGlobalNameStruct::eTypeClassConstructor) { ... but here you don't. Wouldn't you want a if (!name_struct) check inside that first if check? - In nsScriptNamespaceManager.cpp ... + s->mAlias = new nsGlobalNameStruct::ConstructorAlias; + if (!s->mAlias) { + // Free entry + (void) PL_DHashTableOperate(&mGlobalNames, + &NS_ConvertASCIItoUCS2(categoryEntry), + PL_DHASH_REMOVE); + return NS_ERROR_OUT_OF_MEMORY; + } What's up with that silly (void) before the function call? :-) That's not the norm in the dom code :-) Other than that, sr=jst
Attachment #93405 -
Flags: superreview+
Assignee | ||
Comment 5•22 years ago
|
||
Assignee | ||
Comment 6•22 years ago
|
||
*** Bug 138736 has been marked as a duplicate of this bug. ***
Comment on attachment 95990 [details] [diff] [review] v1.1 >@@ -201,6 +224,35 @@ > rv = NS_OK; > > continue; >+ } >+ >+ if (aType == nsGlobalNameStruct::eTypeExternalConstructor) { This is probably me not fully understanding everything, but why do you only want to do this for aType == nsGlobalNameStruct::eTypeExternalConstructor? >+ nsXPIDLCString constructorProto; >+ rv = aCategoryManager->GetCategoryEntry(JAVASCRIPT_GLOBAL_CONSTRUCTOR_PROTO_ALIAS_CATEGORY, >+ categoryEntry, >+ getter_Copies(constructorProto)); >+ if (NS_SUCCEEDED(rv)) { >+ nsGlobalNameStruct *s = AddToHash(NS_ConvertASCIItoUCS2(categoryEntry)); >+ NS_ENSURE_TRUE(s, NS_ERROR_OUT_OF_MEMORY); >+ >+ if (s->mType == nsGlobalNameStruct::eTypeNotInitialized) { >+ s->mAlias = new nsGlobalNameStruct::ConstructorAlias; >+ if (!s->mAlias) { >+ // Free entry >+ PL_DHashTableOperate(&mGlobalNames, >+ &NS_ConvertASCIItoUCS2(categoryEntry), >+ PL_DHASH_REMOVE); >+ return NS_ERROR_OUT_OF_MEMORY; >+ } >+ s->mType = nsGlobalNameStruct::eTypeConstructorAlias; >+ s->mAlias->mCID = cid; >+ s->mAlias->mProtoName.AssignWithConversion(constructorProto); >+ s->mAlias->mProto = nsnull; >+ } else { >+ NS_WARNING("Global script name not overwritten!"); >+ } >+ return NS_OK; >+ } Do you really want to return here? shouldn't you keep looping? Still have some parts of the patch to go...
make sure that the code around http://lxr.mozilla.org/mozilla/source/dom/src/base/nsDOMClassInfo.cpp#3591 doesn't need touching (i don't think it does). A better name for the new enum would probably be eTypeExternalConstructorAlias or eTypeExternalAliasedConstructor Other then that it looks fine
Comment on attachment 95990 [details] [diff] [review] v1.1 >- return catman->AddCategoryEntry(JAVASCRIPT_GLOBAL_CONSTRUCTOR_CATEGORY, >- "Image", NS_HTMLIMGELEMENT_CONTRACTID, >+ nsresult rv = catman->AddCategoryEntry(JAVASCRIPT_GLOBAL_CONSTRUCTOR_CATEGORY, >+ "Image", NS_HTMLIMGELEMENT_CONTRACTID, >+ PR_TRUE, PR_TRUE, getter_Copies(previous)); >+ NS_ENSURE_SUCCESS(rv, rv); >+ >+ return catman->AddCategoryEntry(JAVASCRIPT_GLOBAL_CONSTRUCTOR_PROTO_CATEGORY, >+ "Image", "HTMLImageElement", > PR_TRUE, PR_TRUE, getter_Copies(previous)); This should be JAVASCRIPT_GLOBAL_CONSTRUCTOR_PROTO_ALIAS_CATEGORY (same of option)
Assignee | ||
Comment 10•22 years ago
|
||
Assignee | ||
Comment 11•22 years ago
|
||
> make sure that the code around
> http://lxr.mozilla.org/mozilla/source/dom/src/base/nsDOMClassInfo.cpp#3591
I doesn't because of
+ if (name_struct->mType == nsGlobalNameStruct::eTypeConstructorAlias) {
+ name_struct = alias_struct;
+ }
Assignee | ||
Comment 12•22 years ago
|
||
Use eTypeExternalConstructorAlias instead of eTypeConstructorAlias, use JAVASCRIPT_GLOBAL_CONSTRUCTOR_PROTO_ALIAS_CATEGORY everywhere, continue instead of return in nsScriptNameSpaceManager::FillHash, always return NS_OK at the end of nsScriptNameSpaceManager::FillHash instead of returning rv and resetting it where the errorcode shouldn't leak to the caller.
Attachment #95990 -
Attachment is obsolete: true
Comment on attachment 97237 [details] [diff] [review] v1.2 r=sicking
Attachment #97237 -
Flags: review+
Assignee | ||
Comment 14•22 years ago
|
||
Comment on attachment 97237 [details] [diff] [review] v1.2 Carry over sr=jst.
Attachment #97237 -
Flags: review+ → superreview+
Comment 15•22 years ago
|
||
Comment on attachment 97237 [details] [diff] [review] v1.2 [collision] r=sicking
Attachment #97237 -
Flags: review+
Comment 16•22 years ago
|
||
this broke the SunOS port: "/builds/tinderbox/SeaMonkey/SunOS_5.7_Depend/mozilla/dom/src/build/nsScriptNameSpaceManager.cpp", line 240: Error: The "&" operator can only be applied to a variable or other l-value.
Assignee | ||
Comment 17•22 years ago
|
||
Checked in. (bustage on SunOS fixed too)
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•