[2/2] msxml3/domdoc: Add support for VT_ARRAY|VT_UI1 in domdoc_load().

[Nikolay Sivov]

On 5/3/2011 18:43, Adam Martinson wrote:
> On 05/03/2011 01:45 AM, Nikolay Sivov wrote:
>> On 5/3/2011 08:07, Adam Martinson wrote:
>>>
>>> Fixes bug 14864.
>>> ---
>>>  dlls/msxml3/domdoc.c |  255 
>>> ++++++++++++++++++++++++++------------------------
>>>  1 files changed, 134 insertions(+), 121 deletions(-)
>>>
>>> +    case VT_ARRAY|VT_UI1:
>>> +        {
>>> +            SAFEARRAY* psa;
>>> +            BSTR bstr;
>>> +            psa = V_ARRAY(&source);
>>> +            bstr = bstr_from_xmlChar(psa->pvData);
>>> +            hr = domdoc_loadXML(iface, bstr, isSuccessful);
>>> +            SysFreeString(bstr);
>>> +            return hr;
>>> +        }
>>> +        break;
>> Is it really supposed to be utf8 array? Or this will work cause 
>> one-byte chars match utf8?
>>
>> Also I don't like this method dependency, I think the proper way is 
>> to call IXMLDOMDocument3_loadXML(iface...) here.
>> Or another way to avoid call through vtable - introduce something 
>> called domdoc_load_xml(This...) and use it in both places,
>> but I think it's not worth it.
>
> It was the Google Chrome installer, I'm not sure if it's supposed to 
> be UTF8 or not, all of the characters it used were ASCII.  I'm 
> assuming it's supposed to be UTF8 due to it being VT_UI8 instead of 
> VT_I8, but I can't say for sure.
I'm worried about VT_UI1 being interpreted as ASCII stream or just a 
byte stream that could be in any encoding. If it's a case you can't 
assume it's encoded as utf8, when you do utf8 -> utf16 (for BSTR).

For this particular case you might want direct doparse() call for VT_UI1 
array, I suggest a simple test -- create byte (VT_UI1) array over a 
WCHAR buffer with UTF16 xml data and try to ::load() from it. If 
encoding is detected then you need direct doparse() call, to do 
completely clear case don't include encoding= attribute in this xml data.

>   I remember another place where it would have been useful to be able 
> to call domdoc_loadXML() with a UTF8 string to avoid converting to 
> BSTR and back, I think in the schema stuff.  It would be nice to have 
> an internal function to do that, then domdoc_loadXML() can just wrap 
> it, but I think that can be a separate patch.
Anyway if you need a interface call to implement some other method use 
already defined macro instead of internal static implementation 
function. Internal method implementations better not depend on each other.