Hi,
Just found a bug in mshtml component get_body method implementation. If one call it right after creation of CLSID_HTMLDocument instance, then it would return success and pass NULL as body object. Native would return "empty" body object instead. Native atl.dll library uses this behavior and because of this segfaults. I'm not acquainted with mshtml code enough to provide bug fix or even test patch (I mean "proper" test). Here is quick-n-dirty test instead:
void test_for_body(void) { HRESULT hr; IHTMLDocument *doc; IHTMLDocument2 *doc2; IPersistStreamInit *psi; IHTMLElement *body;
hr = CoCreateInstance(&CLSID_HTMLDocument, NULL, CLSCTX_SERVER, &IID_IHTMLDocument, (void**)&doc); ok( SUCCEEDED(hr), "CoCreateInstance failed:%08x\n", hr); hr = IHTMLDocument_QueryInterface(doc, &IID_IPersistStreamInit, (void**)&psi); ok( SUCCEEDED(hr), "QI PSI failed %08x\n", hr ); IPersistStreamInit_InitNew( psi ); IPersistStreamInit_Release( psi ); hr = IHTMLDocument_QueryInterface(doc, &IID_IHTMLDocument2, (void**)&doc2); IHTMLDocument_Release( doc ); ok( SUCCEEDED(hr), "QI HD2 failed %08x\n", hr ); hr = IHTMLDocument2_get_body( doc2, &body); IHTMLDocument2_Release( doc2 ); ok( SUCCEEDED(hr), "get_body failed %08x\n", hr ); ok( body != NULL, "get_body returned NULL\n" ); if ( body != NULL ) IHTMLElement_Release( body ); }
I hope a man with good mshtml knowledge would easily turn this test into neat small test in few minutes.
Regards, Andrey Turkin
Hi Andrey,
Andrey Turkin wrote:
Hi,
Just found a bug in mshtml component get_body method implementation. If one call it right after creation of CLSID_HTMLDocument instance, then it would return success and pass NULL as body object. Native would return "empty" body object instead. Native atl.dll library uses this behavior and because of this segfaults. I'm not acquainted with mshtml code enough to provide bug fix or even test patch (I mean "proper" test). Here is quick-n-dirty test instead:
void test_for_body(void) { HRESULT hr; IHTMLDocument *doc; IHTMLDocument2 *doc2; IPersistStreamInit *psi; IHTMLElement *body;
hr = CoCreateInstance(&CLSID_HTMLDocument, NULL, CLSCTX_SERVER,&IID_IHTMLDocument, (void**)&doc); ok( SUCCEEDED(hr), "CoCreateInstance failed:%08x\n", hr); hr = IHTMLDocument_QueryInterface(doc, &IID_IPersistStreamInit, (void**)&psi); ok( SUCCEEDED(hr), "QI PSI failed %08x\n", hr ); IPersistStreamInit_InitNew( psi ); IPersistStreamInit_Release( psi ); hr = IHTMLDocument_QueryInterface(doc, &IID_IHTMLDocument2, (void**)&doc2); IHTMLDocument_Release( doc ); ok( SUCCEEDED(hr), "QI HD2 failed %08x\n", hr ); hr = IHTMLDocument2_get_body( doc2, &body); IHTMLDocument2_Release( doc2 ); ok( SUCCEEDED(hr), "get_body failed %08x\n", hr ); ok( body != NULL, "get_body returned NULL\n" ); if ( body != NULL ) IHTMLElement_Release( body ); }
I hope a man with good mshtml knowledge would easily turn this test into neat small test in few minutes.
Thanks for your work on this. Are you sure you have wine_gecko installed? I'm asking because your test works for me. The correct behavior requires quite much work as it depends on a correct loading routine, but I'd expect the currentimplementation to be enough in your case. The background of this problem is:
- current implementation get_body calls Gecko's GetDocument that loads about:blank page in case no page is loaded and returns its body.
- correct implementation get_body return S_OK and NULL if no page is loaded. However IniNew call initializes document so get_body will return a valid body object.
The loading routine is the main problem with current MSHTML, but I'd expect it to be enough for your case.
Thanks, Jacek
Jacek Caban wrote, on 10/20/06 22:13 MSK:
Hi Andrey,
Andrey Turkin wrote:
Hi,
Just found a bug in mshtml component get_body method implementation. If one call it right after creation of CLSID_HTMLDocument instance, then it would return success and pass NULL as body object. Native would return "empty" body object instead. Native atl.dll library uses this behavior and because of this segfaults. I'm not acquainted with mshtml code enough to provide bug fix or even test patch (I mean "proper" test). Here is quick-n-dirty test instead:
void test_for_body(void) { HRESULT hr; IHTMLDocument *doc; IHTMLDocument2 *doc2; IPersistStreamInit *psi; IHTMLElement *body;
hr = CoCreateInstance(&CLSID_HTMLDocument, NULL, CLSCTX_SERVER,&IID_IHTMLDocument, (void**)&doc); ok( SUCCEEDED(hr), "CoCreateInstance failed:%08x\n", hr); hr = IHTMLDocument_QueryInterface(doc, &IID_IPersistStreamInit, (void**)&psi); ok( SUCCEEDED(hr), "QI PSI failed %08x\n", hr ); IPersistStreamInit_InitNew( psi ); IPersistStreamInit_Release( psi ); hr = IHTMLDocument_QueryInterface(doc, &IID_IHTMLDocument2, (void**)&doc2); IHTMLDocument_Release( doc ); ok( SUCCEEDED(hr), "QI HD2 failed %08x\n", hr ); hr = IHTMLDocument2_get_body( doc2, &body); IHTMLDocument2_Release( doc2 ); ok( SUCCEEDED(hr), "get_body failed %08x\n", hr ); ok( body != NULL, "get_body returned NULL\n" ); if ( body != NULL ) IHTMLElement_Release( body ); }
I hope a man with good mshtml knowledge would easily turn this test into neat small test in few minutes.
Thanks for your work on this. Are you sure you have wine_gecko
Right... I have mozctl installed but it weren't picked up by mshtml. I should look better in traces before writing tests :( So, actual problem lies in load_mozctl function in nsembed.c. This function calls load_xpcom() but then falls down and reports failure. I've added one word "return" and things are working now :)
installed? I'm asking because your test works for me. The correct behavior requires quite much work as it depends on a correct loading routine, but I'd expect the currentimplementation to be enough in your case. The background of this problem is:
- current implementation
get_body calls Gecko's GetDocument that loads about:blank page in case no page is loaded and returns its body.
- correct implementation
get_body return S_OK and NULL if no page is loaded. However IniNew call initializes document so get_body will return a valid body object.
The loading routine is the main problem with current MSHTML, but I'd expect it to be enough for your case.
Thanks, Jacek
-
Andrey Turkin -
Jacek Caban