Re: shdocvw 2: Added [get|put]_Visible implementation.

[Robert Shearman]

Jacek Caban wrote:
> static HRESULT WINAPI WebBrowser_put_Visible(IWebBrowser2 *iface, VARIANT_BOOL Value)
> {
>     WebBrowser *This = WEBBROWSER_THIS(iface);
> - FIXME("(%p)->(%x)\n", This, Value);
> - return E_NOTIMPL;
> - VARIANTARG arg;
> - DISPPARAMS dispparams = {&arg, NULL, 1, 0};
> -
> - TRACE("(%p)->(%x)\n", This, Value);
> -
> - This->visible = Value;
> -
> - V_VT(&arg) = VT_BOOL;
> - V_BOOL(&arg) = Value;
> - call_sink(This->doc_host.cp_wbe2, DISPID_ONVISIBLE, &dispparams);
> -
> - return S_OK;

You don't actually show or hide the window, so it would be good to keep 
the FIXME there.

shdocvw 2: Added [get|put]_Visible implementation.

[Jacek Caban]

Robert Shearman wrote:
> Jacek Caban wrote:
> > static HRESULT WINAPI WebBrowser_put_Visible(IWebBrowser2 *iface,
> > VARIANT_BOOL Value)
> > {
> >     WebBrowser *This = WEBBROWSER_THIS(iface);
> > - FIXME("(%p)->(%x)\n", This, Value);
> > - return E_NOTIMPL;
> > - VARIANTARG arg;
> > - DISPPARAMS dispparams = {&arg, NULL, 1, 0};
> > -
> > - TRACE("(%p)->(%x)\n", This, Value);
> > -
> > - This->visible = Value;
> > -
> > - V_VT(&arg) = VT_BOOL;
> > - V_BOOL(&arg) = Value;
> > - call_sink(This->doc_host.cp_wbe2, DISPID_ONVISIBLE, &dispparams);
> > -
> > - return S_OK;
>
> You don't actually show or hide the window, so it would be good to
> keep the FIXME there.

That's all we should do here. This function is implemented in
InternetExplorer object, but not in WebBrowser. See:
http://msdn.microsoft.com/workshop/browser/webbrowser/reference/objects/webb...
http://msdn.microsoft.com/workshop/browser/webbrowser/reference/objects/inte...
and from here:
http://msdn.microsoft.com/workshop/browser/webbrowser/reference/ifaces/iwebb...
"This method only applies when automating Internet Explorer."

I also have tests showing that we do exactly what we should. The same
applies to a few of my other similar patches.

Jacek

shdocvw 2: Added [get|put]_Visible implementation.

[Dimi Paun]

On Sun, 2006-04-30 at 14:51 +0200, Jacek Caban wrote:
> I also have tests showing that we do exactly what we should. The same
> applies to a few of my other similar patches.

This is not as obvious to people, it would be best if you could add
explicit comments in all such cases, to avoid confusion in the future.
BTW, I was about to send the exact same comments Robert sent :)

shdocvw 2: Added [get|put]_Visible implementation.

[Jacek Caban]

Dimi Paun wrote:
> On Sun, 2006-04-30 at 14:51 +0200, Jacek Caban wrote:
> > I also have tests showing that we do exactly what we should. The same
> > applies to a few of my other similar patches.
>
> This is not as obvious to people, it would be best if you could add
> explicit comments in all such cases, to avoid confusion in the future.
> BTW, I was about to send the exact same comments Robert sent :)

I'll send a patch with comments. I'd prefer to add tests as well, but it
has to wait for the switch to MSHTML based WebBrowser implementation
(BTW that seems to be really near).

Jacek

shdocvw 2: Added [get|put]_Visible implementation.

[Dimi Paun]

On Sun, 2006-04-30 at 23:40 +0200, Jacek Caban wrote:
> I'll send a patch with comments. I'd prefer to add tests as well, but
> it has to wait for the switch to MSHTML based WebBrowser
> implementation (BTW that seems to be really near).

Does this means that soon our MSHTML will be strictly better
than Mozilla's ActiveX control?

shdocvw 2: Added [get|put]_Visible implementation.

[Jacek Caban]

Dimi Paun wrote:
> On Sun, 2006-04-30 at 23:40 +0200, Jacek Caban wrote:
> > I'll send a patch with comments. I'd prefer to add tests as well, but
> > it has to wait for the switch to MSHTML based WebBrowser
> > implementation (BTW that seems to be really near).
>
> Does this means that soon our MSHTML will be strictly better
> than Mozilla's ActiveX control?

Well, IMO it is better now, but it depends on criterion. Mozilla ActiveX
control has more functionality implemented, but their compatibility with
native WebBrowser is quite bad. They seem to believe MSDN... Everything
I wrote was tested and compared to windows version so our compatibility
should be quite good. Also we have the correct architecture while
Mozilla doesn't. For someone not interested in insides of those
libraries probably the more important thing is what and how works. After
the switch to our implementation bugs 3466 and 3515 will be fixed. In
both cases there are still some problems, but apps can run and view HTML.

If someone wants to test it, I've created a Wiki page about it:
http://wiki.winehq.org/ShDocVw.

Because AFAIK there is much interest in Steam, I plan to do the switch
as follows:
- Make Steam work perfectly (I mean its HTML part)
- Do some testing and improvements to prevent regressions
- Finally get rid of Mozilla ActiveX control dependency

But it will have to wait until I pass my exams...

Jacek