User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:18.104.22.168) Gecko/20060508 Firefox/22.214.171.124 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.
Created attachment 228770 [details] [diff] [review] Make SetAsDOMString set the type to nsIDataType::VTYPE_DOMSTRING
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
Created attachment 228889 [details] [diff] [review] Implement SetAsDOMString so it sets the correct type New attachment copies the implementation, as suggested by firstname.lastname@example.org
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+
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
Last Resolved: 12 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
> <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.