On Sun, Jan 23, 2011 at 11:32 AM, Jacek Caban <ja...@codeweavers.com> wrote:
> On 1/20/11 6:40 PM, Erich Hoover wrote:
>> On Thu, Jan 20, 2011 at 4:00 AM, Jacek Caban<ja...@codeweavers.com> wrote:
>>> + ok(status& success_flag, "OLECMDID_STOP not enabled/supported:
>>> %08x\n", status);
>>> You could test the exact value here: ok(status == ...)
>>
>> I was trying to avoid adding some conditional todo_wine calls
>
> Hiding differences between Wine and Windows in tests is the worst thing you
> can do. When one is trying to find a bug, tested parts of implementation are
> much less suspicious than not or poorly tested. If you know about more such
> things in the patch, please fix them. You don't write the test to pass on
> your implementation. You write it to know the right implementation.
I would say that testing the mask is not about hiding differences, it
makes it clear that this particular test is meant to demonstrate
whether a specific feature is available. At some point you have to
make a trade-off between making a particular test completely
comprehensive or having it be clear exactly what is being tested. In
this particular component there are a lot of things that are not yet
implemented, so it's a little difficult to add a self-contained test
for a feature without it ballooning into a bunch of unrelated issues.
There are two other such "issues" I have become aware of with the
current implementation that could be added as tests, but I would say
that they both belong in separate patches (if you want them at all):
1) The IOleCommandTarget is cached when the WebBrowser is initialized
(rather than being requested each time it is used)
2) The document is initialized much earlier on native than it is in Wine
My goal here is to implement (as well as I can) a single feature that
I've had sitting in my stack of unsubmitted patches, I'm not trying to
implement everything in this component in one fell swoop. If that's
really what it's going to take to get this feature implemented then I
can take the time to implement more, but it'll take me a while and I'd
really like to move this item off of my plate so I can clean up some
of my other unsubmitted patches before they become too far out of
sync.
>> (for some reason native does not respond with "supported", even though it
>> does in the mshtml tests), please see if you like the attached better.
> Or it proves that it's not a simple forward to mshtml?
It's possible that they masked the supported flag out, especially
since all of the examples I've seen on MSDN check for the enabled flag
rather than the supported flag. However, I doubt that they
copy-pasted the mshtml code and then made changes to it. I'd say that
even if they did it'd be better to forward to mshtml and discover if
there are any differences (and where they are) as people test
applications that use this functionality.
> We still have them in separated functions you've called test_QueryStatusWB
> and test_ExecWB. We most likely will want more tests on WebBrowser instance
> without container's IOleCommandTarget, and it would be nice if we could reuse
> test_CommandTargetPassthru for that (well, the name would be better more
> generic as well).
Sorry, for some reason I was a bit dense and didn't catch exactly what
you were planning here. Please see if the attached is more to your
liking, the test bot results are available here:
https://testbot.winehq.org/JobDetails.pl?Key=8575
Erich Hoover
ehoo...@mines.edu
diff --git a/dlls/shdocvw/tests/webbrowser.c b/dlls/shdocvw/tests/webbrowser.c
index afde2e1..85a7c3a 100644
--- a/dlls/shdocvw/tests/webbrowser.c
+++ b/dlls/shdocvw/tests/webbrowser.c
@@ -122,6 +122,7 @@ DEFINE_EXPECT(Exec_SETPROGRESSPOS);
DEFINE_EXPECT(Exec_UPDATECOMMANDS);
DEFINE_EXPECT(Exec_SETTITLE);
DEFINE_EXPECT(QueryStatus_SETPROGRESSTEXT);
+DEFINE_EXPECT(Exec_STOP);
DEFINE_EXPECT(QueryStatus_STOP);
DEFINE_EXPECT(DocHost_EnableModeless_TRUE);
DEFINE_EXPECT(DocHost_EnableModeless_FALSE);
@@ -138,10 +139,11 @@ static const WCHAR wszItem[] = {'i','t','e','m',0};
static const WCHAR emptyW[] = {0};
static VARIANT_BOOL exvb;
+
static IWebBrowser2 *wb;
static HWND container_hwnd, shell_embedding_hwnd;
-static BOOL is_downloading, is_first_load;
+static BOOL is_downloading, is_first_load, use_container_olecmd;
static HRESULT hr_dochost_TranslateAccelerator = E_NOTIMPL;
static HRESULT hr_site_TranslateAccelerator = E_NOTIMPL;
static const char *current_url;
@@ -197,6 +199,17 @@ static BSTR a2bstr(const char *str)
return ret;
}
+#define create_WebBrowser(a) _create_WebBrowser(__LINE__,a)
+static HRESULT _create_WebBrowser(unsigned line, IUnknown **unk)
+{
+ HRESULT hres;
+
+ hres = CoCreateInstance(&CLSID_WebBrowser, NULL, CLSCTX_INPROC_SERVER|CLSCTX_INPROC_HANDLER,
+ &IID_IUnknown, (void**)unk);
+ ok_(__FILE__,line)(hres == S_OK, "Creating WebBrowser object failed: %08x\n", hres);
+ return hres;
+}
+
#define test_LocationURL(a,b) _test_LocationURL(__LINE__,a,b)
static void _test_LocationURL(unsigned line, IUnknown *unk, const char *exurl)
{
@@ -308,6 +321,10 @@ static HRESULT WINAPI OleCommandTarget_QueryStatus(IOleCommandTarget *iface, con
CHECK_EXPECT(QueryStatus_SETPROGRESSTEXT);
prgCmds[0].cmdf = OLECMDF_ENABLED;
return S_OK;
+ case IDM_STOP:
+ CHECK_EXPECT(QueryStatus_STOP);
+ prgCmds[0].cmdf = 0;
+ return S_OK;
default:
ok(0, "unexpected command %d\n", prgCmds[0].cmdID);
}
@@ -366,6 +383,12 @@ static HRESULT WINAPI OleCommandTarget_Exec(IOleCommandTarget *iface, const GUID
CHECK_EXPECT(Exec_SETTITLE);
/* TODO: test args */
return S_OK;
+ case OLECMDID_STOP:
+ CHECK_EXPECT(Exec_STOP);
+ return S_OK;
+ case IDM_STOP:
+ CHECK_EXPECT(Exec_STOP);
+ return OLECMDERR_E_NOTSUPPORTED;
default:
ok(0, "unexpected nsCmdID %d\n", nCmdID);
}
@@ -428,8 +451,13 @@ static HRESULT WINAPI OleContainer_QueryInterface(IOleContainer *iface, REFIID r
return E_NOINTERFACE; /* TODO */
if(IsEqualGUID(&IID_IOleCommandTarget, riid)) {
- *ppv = &OleCommandTarget;
- return S_OK;
+ if(use_container_olecmd)
+ {
+ *ppv = &OleCommandTarget;
+ return S_OK;
+ }
+ else
+ return E_NOINTERFACE;
}
ok(0, "unexpected call\n");
@@ -1729,7 +1757,7 @@ static void test_ClientSite(IUnknown *unk, IOleClientSite *client, BOOL stop_dow
SET_EXPECT(Invoke_AMBIENT_SILENT);
}else if(stop_download) {
SET_EXPECT(Invoke_DOWNLOADCOMPLETE);
- SET_EXPECT(Exec_SETDOWNLOADSTATE_0);
+ if (use_container_olecmd) SET_EXPECT(Exec_SETDOWNLOADSTATE_0);
SET_EXPECT(Invoke_COMMANDSTATECHANGE);
}
@@ -1743,7 +1771,7 @@ static void test_ClientSite(IUnknown *unk, IOleClientSite *client, BOOL stop_dow
CHECK_CALLED(Invoke_AMBIENT_SILENT);
}else if(stop_download) {
todo_wine CHECK_CALLED(Invoke_DOWNLOADCOMPLETE);
- todo_wine CHECK_CALLED(Exec_SETDOWNLOADSTATE_0);
+ if (use_container_olecmd) todo_wine CHECK_CALLED(Exec_SETDOWNLOADSTATE_0);
todo_wine CHECK_CALLED(Invoke_COMMANDSTATECHANGE);
}
@@ -2343,7 +2371,7 @@ static void test_Navigate2(IUnknown *unk)
SET_EXPECT(Invoke_PROPERTYCHANGE);
SET_EXPECT(Invoke_BEFORENAVIGATE2);
SET_EXPECT(Invoke_DOWNLOADBEGIN);
- SET_EXPECT(Exec_SETDOWNLOADSTATE_1);
+ if (use_container_olecmd) SET_EXPECT(Exec_SETDOWNLOADSTATE_1);
SET_EXPECT(EnableModeless_FALSE);
SET_EXPECT(Invoke_STATUSTEXTCHANGE);
SET_EXPECT(SetStatusText);
@@ -2353,14 +2381,15 @@ static void test_Navigate2(IUnknown *unk)
SET_EXPECT(Invoke_AMBIENT_PALETTE);
SET_EXPECT(GetOptionKeyPath);
SET_EXPECT(GetOverridesKeyPath);
- SET_EXPECT(QueryStatus_SETPROGRESSTEXT);
- SET_EXPECT(Exec_SETPROGRESSMAX);
- SET_EXPECT(Exec_SETPROGRESSPOS);
+ if (use_container_olecmd) SET_EXPECT(QueryStatus_SETPROGRESSTEXT);
+ if (use_container_olecmd) SET_EXPECT(Exec_SETPROGRESSMAX);
+ if (use_container_olecmd) SET_EXPECT(Exec_SETPROGRESSPOS);
SET_EXPECT(Invoke_SETSECURELOCKICON);
SET_EXPECT(Invoke_FILEDOWNLOAD);
- SET_EXPECT(Exec_SETDOWNLOADSTATE_0);
+ if (use_container_olecmd) SET_EXPECT(Exec_SETDOWNLOADSTATE_0);
SET_EXPECT(Invoke_COMMANDSTATECHANGE);
SET_EXPECT(EnableModeless_TRUE);
+ if (!use_container_olecmd) SET_EXPECT(Invoke_DOWNLOADCOMPLETE);
}
hres = IWebBrowser2_Navigate2(webbrowser, &url, NULL, NULL, NULL, NULL);
@@ -2371,7 +2400,7 @@ static void test_Navigate2(IUnknown *unk)
todo_wine CHECK_CALLED(Invoke_PROPERTYCHANGE);
CHECK_CALLED(Invoke_BEFORENAVIGATE2);
todo_wine CHECK_CALLED(Invoke_DOWNLOADBEGIN);
- todo_wine CHECK_CALLED(Exec_SETDOWNLOADSTATE_1);
+ if (use_container_olecmd) todo_wine CHECK_CALLED(Exec_SETDOWNLOADSTATE_1);
CHECK_CALLED(EnableModeless_FALSE);
CHECK_CALLED(Invoke_STATUSTEXTCHANGE);
CHECK_CALLED(SetStatusText);
@@ -2381,13 +2410,13 @@ static void test_Navigate2(IUnknown *unk)
CHECK_CALLED(Invoke_AMBIENT_PALETTE);
CHECK_CALLED(GetOptionKeyPath);
CHECK_CALLED(GetOverridesKeyPath);
- todo_wine CHECK_CALLED(QueryStatus_SETPROGRESSTEXT);
- todo_wine CHECK_CALLED(Exec_SETPROGRESSMAX);
- todo_wine CHECK_CALLED(Exec_SETPROGRESSPOS);
+ if (use_container_olecmd) todo_wine CHECK_CALLED(QueryStatus_SETPROGRESSTEXT);
+ if (use_container_olecmd) todo_wine CHECK_CALLED(Exec_SETPROGRESSMAX);
+ if (use_container_olecmd) todo_wine CHECK_CALLED(Exec_SETPROGRESSPOS);
todo_wine CHECK_CALLED(Invoke_SETSECURELOCKICON);
todo_wine CHECK_CALLED(Invoke_FILEDOWNLOAD);
todo_wine CHECK_CALLED(Invoke_COMMANDSTATECHANGE);
- todo_wine CHECK_CALLED(Exec_SETDOWNLOADSTATE_0);
+ if (use_container_olecmd) todo_wine CHECK_CALLED(Exec_SETDOWNLOADSTATE_0);
CHECK_CALLED(EnableModeless_TRUE);
}
@@ -2397,6 +2426,70 @@ static void test_Navigate2(IUnknown *unk)
test_ready_state(READYSTATE_LOADING);
}
+static void test_QueryStatusWB(IWebBrowser2 *webbrowser, BOOL use_custom_target, BOOL has_document)
+{
+ HRESULT hres, success_state;
+ OLECMDF success_flags;
+ enum OLECMDF status;
+
+ if (use_custom_target)
+ success_flags = OLECMDF_SUPPORTED;
+ else
+ success_flags = OLECMDF_ENABLED;
+
+ if (has_document)
+ success_state = S_OK;
+ else
+ success_state = E_UNEXPECTED;
+
+ /* Test a safe operation that exists as both a high-numbered MSHTML id and an OLECMDID */
+ status = 0xdeadbeef;
+ if (use_custom_target) SET_EXPECT(QueryStatus_STOP);
+ hres = IWebBrowser2_QueryStatusWB(webbrowser, OLECMDID_STOP, &status);
+ ok(hres == success_state, "QueryStatusWB failed: %08x %08x\n", hres, success_state);
+ if (!use_custom_target && has_document)
+ todo_wine ok((has_document && status == success_flags) || (!has_document && status == 0xdeadbeef),
+ "OLECMDID_STOP not enabled/supported: %08x %08x\n", status, success_flags);
+ else
+ ok((has_document && status == success_flags) || (!has_document && status == 0xdeadbeef),
+ "OLECMDID_STOP not enabled/supported: %08x %08x\n", status, success_flags);
+ status = 0xdeadbeef;
+ if (use_custom_target) SET_EXPECT(QueryStatus_STOP);
+ hres = IWebBrowser2_QueryStatusWB(webbrowser, IDM_STOP, &status);
+ ok(hres == success_state, "QueryStatusWB failed: %08x %08x\n", hres, success_state);
+ ok((has_document && status == 0) || (!has_document && status == 0xdeadbeef),
+ "IDM_STOP is enabled/supported: %08x %d\n", status, has_document);
+}
+
+static void test_ExecWB(IWebBrowser2 *webbrowser, BOOL use_custom_target, BOOL has_document)
+{
+ HRESULT hres, success_state, fail_state;
+
+ if (has_document)
+ {
+ success_state = S_OK;
+ fail_state = OLECMDERR_E_NOTSUPPORTED;
+ }
+ else
+ {
+ success_state = E_UNEXPECTED;
+ fail_state = E_UNEXPECTED;
+ }
+
+ /* Test a safe operation that exists as both a high-numbered MSHTML id and an OLECMDID */
+ if (use_custom_target)
+ SET_EXPECT(Exec_STOP);
+ hres = IWebBrowser2_ExecWB(webbrowser, OLECMDID_STOP, OLECMDEXECOPT_DONTPROMPTUSER, 0, 0);
+ if (!use_custom_target && has_document)
+ todo_wine ok(hres == success_state, "ExecWB failed: %08x %08x\n", hres, success_state);
+ else
+ ok(hres == success_state, "ExecWB failed: %08x %08x\n", hres, success_state);
+ if (use_custom_target)
+ SET_EXPECT(Exec_STOP);
+ hres = IWebBrowser2_ExecWB(webbrowser, IDM_STOP, OLECMDEXECOPT_DONTPROMPTUSER, 0, 0);
+ ok(hres == fail_state, "ExecWB failed: %08x %08x\n", hres, fail_state);
+}
+
static void test_download(DWORD flags)
{
MSG msg;
@@ -2848,22 +2941,24 @@ static void test_dochost_qs(IUnknown *unk)
IServiceProvider_Release(serv_prov);
}
-static void test_WebBrowser(BOOL do_download)
+static void test_WebBrowser_FullFeatured(BOOL do_download)
{
IUnknown *unk = NULL;
ULONG ref;
HRESULT hres;
- hres = CoCreateInstance(&CLSID_WebBrowser, NULL, CLSCTX_INPROC_SERVER|CLSCTX_INPROC_HANDLER,
- &IID_IUnknown, (void**)&unk);
- ok(hres == S_OK, "Creating WebBrowser object failed: %08x\n", hres);
+ if (FAILED(create_WebBrowser(&unk)))
+ return;
is_downloading = FALSE;
is_first_load = TRUE;
+ use_container_olecmd = TRUE;
hres = IUnknown_QueryInterface(unk, &IID_IWebBrowser2, (void**)&wb);
ok(hres == S_OK, "Could not get IWebBrowser2 iface: %08x\n", hres);
+ test_QueryStatusWB(wb, FALSE, FALSE);
+ test_ExecWB(wb, FALSE, FALSE);
test_QueryInterface(unk);
test_ready_state(READYSTATE_UNINITIALIZED);
test_ClassInfo(unk);
@@ -2875,6 +2970,8 @@ static void test_WebBrowser(BOOL do_download)
test_DoVerb(unk);
test_olecmd(unk, FALSE);
test_Navigate2(unk);
+ test_QueryStatusWB(wb, TRUE, TRUE);
+ test_ExecWB(wb, TRUE, TRUE);
if(do_download) {
IDispatch *doc, *doc2;
@@ -2912,6 +3009,40 @@ static void test_WebBrowser(BOOL do_download)
ok(ref == 0, "ref=%d, expected 0\n", ref);
}
+static void test_WebBrowser_NoContainerOlecmd(void)
+{
+ IUnknown *unk = NULL;
+ HRESULT hres;
+ ULONG ref;
+
+ is_downloading = FALSE;
+ is_first_load = TRUE;
+ use_container_olecmd = FALSE;
+
+ /* Setup stage */
+ if (FAILED(create_WebBrowser(&unk)))
+ return;
+ test_ConnectionPoint(unk, TRUE);
+ test_ClientSite(unk, &ClientSite, TRUE);
+ test_DoVerb(unk);
+ test_Navigate2(unk);
+ hres = IUnknown_QueryInterface(unk, &IID_IWebBrowser2, (void**)&wb);
+ ok(hres == S_OK, "QueryInterface(IID_IWebBrowser) failed: %08x\n", hres);
+ if(FAILED(hres))
+ return;
+
+ /* Tests of interest */
+ test_QueryStatusWB(wb, FALSE, TRUE);
+ test_ExecWB(wb, FALSE, TRUE);
+
+ /* Cleanup stage */
+ IWebBrowser2_Release(wb);
+ test_ClientSite(unk, NULL, TRUE);
+ test_ConnectionPoint(unk, FALSE);
+ ref = IUnknown_Release(unk);
+ ok(ref == 0, "ref=%d, expected 0\n", ref);
+}
+
static BOOL check_ie(void)
{
IHTMLDocument5 *doc;
@@ -2933,10 +3064,12 @@ START_TEST(webbrowser)
if(check_ie()) {
container_hwnd = create_container_window();
- trace("Testing WebBrowser (no download)...\n");
- test_WebBrowser(FALSE);
- trace("Testing WebBrowser...\n");
- test_WebBrowser(TRUE);
+ trace("Testing Full-Featured WebBrowser (no download)...\n");
+ test_WebBrowser_FullFeatured(FALSE);
+ trace("Testing Full-Featured WebBrowser...\n");
+ test_WebBrowser_FullFeatured(TRUE);
+ trace("Testing WebBrowser w/o container-based olecmd...\n");
+ test_WebBrowser_NoContainerOlecmd();
}else {
win_skip("Skipping tests on too old IE\n");
}
diff --git a/dlls/shdocvw/webbrowser.c b/dlls/shdocvw/webbrowser.c
index 1a0e189..a0a8020 100644
--- a/dlls/shdocvw/webbrowser.c
+++ b/dlls/shdocvw/webbrowser.c
@@ -780,16 +780,75 @@ static HRESULT WINAPI WebBrowser_Navigate2(IWebBrowser2 *iface, VARIANT *URL, VA
static HRESULT WINAPI WebBrowser_QueryStatusWB(IWebBrowser2 *iface, OLECMDID cmdID, OLECMDF *pcmdf)
{
WebBrowser *This = impl_from_IWebBrowser2(iface);
- FIXME("(%p)->(%d %p)\n", This, cmdID, pcmdf);
- return E_NOTIMPL;
+ IOleCommandTarget *target = NULL;
+ OLECMD ole_command[1];
+ HRESULT hres;
+
+ TRACE("(%p)->(%d %p)\n", This, cmdID, pcmdf);
+
+ if (!pcmdf)
+ return E_POINTER;
+ ole_command[0].cmdID = cmdID;
+ ole_command[0].cmdf = *pcmdf;
+
+ if (This->container)
+ {
+ hres = IOleContainer_QueryInterface(This->container, &IID_IOleCommandTarget, (LPVOID*)&target);
+ if(FAILED(hres))
+ target = NULL;
+ }
+ if (!target && This->doc_host.document)
+ {
+ hres = IOleContainer_QueryInterface(This->doc_host.document, &IID_IOleCommandTarget, (LPVOID*)&target);
+ if(FAILED(hres))
+ target = NULL;
+ }
+
+ if (!target)
+ return E_UNEXPECTED;
+
+ hres = IOleCommandTarget_QueryStatus(target, NULL, 1, ole_command, NULL);
+ if (SUCCEEDED(hres))
+ *pcmdf = ole_command[0].cmdf;
+ if (hres == OLECMDERR_E_NOTSUPPORTED)
+ {
+ *pcmdf = 0;
+ hres = S_OK;
+ }
+ IOleCommandTarget_Release(target);
+
+ return hres;
}
static HRESULT WINAPI WebBrowser_ExecWB(IWebBrowser2 *iface, OLECMDID cmdID,
OLECMDEXECOPT cmdexecopt, VARIANT *pvaIn, VARIANT *pvaOut)
{
WebBrowser *This = impl_from_IWebBrowser2(iface);
- FIXME("(%p)->(%d %d %s %p)\n", This, cmdID, cmdexecopt, debugstr_variant(pvaIn), pvaOut);
- return E_NOTIMPL;
+ IOleCommandTarget *target = NULL;
+ HRESULT hres;
+
+ TRACE("(%p)->(%d %d %s %p)\n", This, cmdID, cmdexecopt, debugstr_variant(pvaIn), pvaOut);
+
+ if(This->container)
+ {
+ hres = IOleContainer_QueryInterface(This->container, &IID_IOleCommandTarget, (LPVOID*)&target);
+ if(FAILED(hres))
+ target = NULL;
+ }
+ if(!target && This->doc_host.document)
+ {
+ hres = IOleContainer_QueryInterface(This->doc_host.document, &IID_IOleCommandTarget, (LPVOID*)&target);
+ if(FAILED(hres))
+ target = NULL;
+ }
+
+ if(!target)
+ return E_UNEXPECTED;
+
+ hres = IOleCommandTarget_Exec(target, NULL, cmdID, cmdexecopt, pvaIn, pvaOut);
+ IOleCommandTarget_Release(target);
+
+ return hres;
}
static HRESULT WINAPI WebBrowser_ShowBrowserBar(IWebBrowser2 *iface, VARIANT *pvaClsid,
