Closed
Bug 575345
Opened 15 years ago
Closed 15 years ago
Brower generates "Did you mean to access that lowercase property" message for declarations like "var FullScreen"
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: dmandelin, Assigned: dmandelin)
References
Details
Spun off from bug 574745 comment 5.
Assignee | ||
Comment 1•15 years ago
|
||
* Detailed Cause
OK, I think I found out what's going on here. The error is actually generated by the statement 'var FullScreen = ...', specifically the DEFVAR. The DEFVAR does a lookup to see if the property is already defined, in which case there is an error.
That lookup fails on its last chance, XPC_WN_ModsAllowed_Proto_Resolve. That function then calls DefinePropertyIfFound, which calls HandlePossibleNameCaseError. That function in turn calls XPCNativeSet::FindMember on the lower-case version of the name, 'fullScreen'.
If FindMember returns true, then we generate the error message, but otherwise not.
In TM (and m-c), FindMember returns false, but in fatvals, FindMember returns true. It appears that TM/m-c are the ones with the bug: in the final comparison function they use to find the property, they call XPCNativeInterface::FindMember, which is:
// Tracemonkey/mozilla-central
inline XPCNativeMember*
XPCNativeInterface::FindMember(jsval name) const
{
const XPCNativeMember* member = mMembers;
for(int i = (int) mMemberCount; i > 0; i--, member++)
if(member->GetName() == name)
return const_cast<XPCNativeMember*>(member);
return nsnull;
}
Note that two string jsvals are being compared using '=='. This is wrong unless the strings are interned; otherwise two strings of the same value could compare unequal. fatvals does it right, comparing two jsids, which are required to be interned if strings.
* Conclusions
Now, it's fairly clear that merely declaring a name shouldn't produce an error like this. I'm not sure exactly what step in the chain is wrong. I guess we probably do want to call the resolve hook (after all, we have JSRESOLVE_DECLARING), so the error handling in XPConnect must be to blame. Should I just shut off calling HandlePossibleNameCaseError if we have JSRESOLVE_DECLARING?
Assignee | ||
Updated•15 years ago
|
Summary: fatvals: JS errors starting up browser. → Brower generates "Did you mean to access that lowercase property" message for declarations like "var FullScreen"
![]() |
||
Comment 2•15 years ago
|
||
Specifically, fatval fixes a bug where a non-intern string was being stuffed into a jsval (which of course doesn't assert isAtomized) and then implicitly converted jsid.
Comment 3•15 years ago
|
||
(In reply to comment #1)
> Should I just shut off calling HandlePossibleNameCaseError if we have
> JSRESOLVE_DECLARING?
Yes, or even in all cases, tbh. I think it has outlived its utility.
![]() |
||
Comment 4•15 years ago
|
||
I'm seeing the warning show up a lot in automated test spew. I think this will annoy everyone so I'm taking comment 3 as a go-ahead to remove the check in fatvals.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•