Re: [PATCH] mshtml: implement ActiveScriptSite_OnScriptError.

[Jacek Caban]

On 9/29/10 9:39 AM, Reece Dunn wrote:
> Hi,
>
> This reports any errors sent to the mshtml ActiveScriptSite
> OnScriptError handler to the user and traces it to ERR to aid
> debugging.

static HRESULT WINAPI ActiveScriptSite_OnScriptError(IActiveScriptSite *iface, IActiveScriptError *pscripterror)
  {
      ScriptHost *This = ACTSCPSITE_THIS(iface);
-    FIXME("(%p)->(%p)\n", This, pscripterror);
-    return E_NOTIMPL;
+    EXCEPINFO excep;
+    HRESULT hr;
+
+    TRACE("(%p)->(%p)\n", This, pscripterror);
+
+    hr = IActiveScriptError_GetExceptionInfo(pscripterror,&excep);
+
+    ERR("ActiveScript Error: %s source: %s\n",
+        debugstr_w(excep.bstrDescription), debugstr_w(excep.bstrSource));

ERR is not appropriate in this case.

+    MessageBoxW(NULL, excep.bstrDescription, excep.bstrSource, MB_OK|MB_ICONERROR);

This is not how this function should work. If you want to implement it, please start with a test case. It probably should fire onerror event.

Jacek

[PATCH] mshtml: implement ActiveScriptSite_OnScriptError.

[Reece Dunn]

On 29 September 2010 11:41, Jacek Caban  wrote:
> On 9/29/10 9:39 AM, Reece Dunn wrote:
> > Hi,
> >
> > This reports any errors sent to the mshtml ActiveScriptSite
> > OnScriptError handler to the user and traces it to ERR to aid
> > debugging.
>
> static HRESULT WINAPI ActiveScriptSite_OnScriptError(IActiveScriptSite
> *iface, IActiveScriptError *pscripterror)
>  {
>     ScriptHost *This = ACTSCPSITE_THIS(iface);
> - FIXME("(%p)->(%p)\n", This, pscripterror);
> - return E_NOTIMPL;
> - EXCEPINFO excep;
> - HRESULT hr;
> -
> - TRACE("(%p)->(%p)\n", This, pscripterror);
> -
> - hr = IActiveScriptError_GetExceptionInfo(pscripterror,&excep);
> -
> - ERR("ActiveScript Error: %s source: %s\n",
> - debugstr_w(excep.bstrDescription), debugstr_w(excep.bstrSource));
>
> ERR is not appropriate in this case.
> - MessageBoxW(NULL, excep.bstrDescription, excep.bstrSource,
>
> MB_OK|MB_ICONERROR);
>
> This is not how this function should work. If you want to implement it,
> please start with a test case. It probably should fire onerror event.

OK. I will look into addressing this with an associated test case.

- Reece