Closed
Bug 344196
Opened 18 years ago
Closed 18 years ago
nsVariant::SetAsDOMString sets type to nsIDataType::VTYPE_ASTRING not VTYPE_DOMSTRING
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
People
(Reporter: andrew, Assigned: andrew)
Details
Attachments
(1 file, 1 obsolete file)
1.18 KB,
patch
|
dougt
:
review+
brendan
:
superreview+
dbaron
:
approval1.8.1+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4
Build Identifier: Latest CVS trunk
The nsVariant::SetAsDOMString function (which implements part of nsIWritableVariant) delegates the work to SetFromAString, but then doesn't override the type back to nsIDataType::VTYPE_DOMSTRING. This means that there is no way to set a VTYPE_DOMSTRING except through XPConnect.
Reproducible: Always
Steps to Reproduce:
Sample code which demonstrates the problem:
nsString drawColour;
ret = aDTCR->GetDrawColour(drawColour);
NS_ENSURE_SUCCESS(ret,);
mDrawColour = do_CreateInstance(NS_VARIANT_CONTRACTID, &ret);
NS_ENSURE_SUCCESS(ret,);
mDrawColour->SetAsDOMString(drawColour);
PRUint16 dt;
mDrawColour->GetDataType(&dt);
NS_ASSERTION(dt == nsIDataType::VTYPE_DOMSTRING,
"Buggy nsIVariant set wrong type");
Actual Results:
###!!! ASSERTION: Buggy nsIVariant set wrong type: 'dt == nsIDataType::VTYPE_DOMSTRING', file DataCollector/DataCollector.cpp, line 34
Expected Results:
The assertion should not trip.
Assignee | ||
Comment 1•18 years ago
|
||
Attachment #228770 -
Flags: review?(dougt)
Comment 2•18 years ago
|
||
sounds like the right thing to do. Maybe you want to copy SetFromAString() into this method to avoid assigning mType twice?
Assignee: nobody → ak.miller
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Updated•18 years ago
|
Attachment #228770 -
Flags: review?(dougt)
Assignee | ||
Comment 3•18 years ago
|
||
New attachment copies the implementation, as suggested by dougt@meer.net
Attachment #228770 -
Attachment is obsolete: true
Attachment #228889 -
Flags: review?(dougt)
Updated•18 years ago
|
Attachment #228889 -
Flags: review?(dougt) → review+
Comment 4•18 years ago
|
||
Comment on attachment 228889 [details] [diff] [review]
Implement SetAsDOMString so it sets the correct type
sr=me (requested over IRC, please use the request flag feature in the future).
/be
Attachment #228889 -
Flags: superreview+
Assignee | ||
Updated•18 years ago
|
Whiteboard: [checkin needed]
Updated•18 years ago
|
Status: NEW → ASSIGNED
Comment 5•18 years ago
|
||
Checking in xpcom/ds/nsVariant.cpp;
/cvsroot/mozilla/xpcom/ds/nsVariant.cpp,v <-- nsVariant.cpp
new revision: 1.29; previous revision: 1.28
done
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Assignee | ||
Updated•18 years ago
|
Attachment #228889 -
Flags: approval1.8.1?
Assignee | ||
Comment 6•18 years ago
|
||
> <reed> A1kmm: Make sure you leave a comment about (any) risks involved in taking a patch on the branch and what exactly it does.
The patch makes nsVariant::SetAsDOMString set the type to VTYPE_DOMSTRING (it previously incorrectly set it to VTYPE_ASTRING).
The patch is relatively safe, because nothing in the standard tree actually calls this method, and it can only be called from C++ (therefore, it will make a difference to extensions).
The patch is especially worth having on the branch because it fixes something to behave consistently, and it otherwise would be hard to use canvas from C++.
Comment on attachment 228889 [details] [diff] [review]
Implement SetAsDOMString so it sets the correct type
Seems a little dangerous in case somebody's depending on the bug, but that seems relatively unlikely.
a=dbaron on behalf of drivers. Please land on MOZILLA_1_8_BRANCH and add the fixed1.8.1 keyword once you have done so.
Attachment #228889 -
Flags: approval1.8.1? → approval1.8.1+
You need to log in
before you can comment on or make changes to this bug.
Description
•