[webkit-reviews] review denied: [Bug 80760] WinLauncher should show loading errors : [Attachment 131250] Patch

[bugzilla-daemon at webkit.org]

Adam Roben (:aroben) <aroben at webkit.org> has denied Ashod Nakashian
<ashodnakashian at yahoo.com>'s request for review:
Bug 80760: WinLauncher should show loading errors
https://bugs.webkit.org/show_bug.cgi?id=80760

Attachment 131250: Patch
https://bugs.webkit.org/attachment.cgi?id=131250&action=review

------- Additional Comments from Adam Roben (:aroben) <aroben at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=131250&action=review


> Tools/WinLauncher/WinLauncher.cpp:152
> +    ::MessageBoxW(0, static_cast<LPCWSTR>(errorDescription), L"Error",
MB_APPLMODAL | MB_OK);

Using a message box seems awfully heavy-handed. Is there some less disruptive
way of notifying the user?

> Tools/WinLauncher/WinLauncher.h:53
>      virtual HRESULT STDMETHODCALLTYPE didFailProvisionalLoadWithError( 
>	   /* [in] */ IWebView *webView,
>	   /* [in] */ IWebError *error,
> -	   /* [in] */ IWebFrame *frame) { return S_OK; }
> +	   /* [in] */ IWebFrame *frame) { return
didFailProvisionalLoadWithError(webView, error); }

I don't think the extra overload of didFailProvisionalLoadWithError is needed.
I'd just move the implementation of this overload to the .cpp file and put the
MessageBox code there.