In WebKit the error consists of three components: 1) The domain 2) The domain specific error code 3) A (localized) description This is visible in the public WK2 API, it is also used as internal representation and it maps exactly to how NSError in the public ObjC WebKit API works. The current QQuickWebView API exposes the domain and error code through a signal and an enum: enum ErrorType { EngineError, NetworkError, HttpError, DownloadError }; void loadFailed(QQuickWebView::ErrorType errorType, int errorCode, const QUrl& url); I suggest we adapt our API to the WebKit/NSError way by changing the error type to a domain string. In addition we need to document the domains as well as some domain specific error codes. Perhaps we should also expose the localized description? Let's discuss here.
I suggest to rename errorType also to errorDomain and use WebView.HTTPErrorDomain for example, so that the reads: onLoadingFailed: { if (errorDomain = WebView.HTTPErrorDomain) { if (errorCode == 401) ... // interpret errorCode as http error code } }
Created attachment 119077 [details] Patch
(In reply to comment #2) > Created an attachment (id=119077) [details] > Patch I'm not asking for review yet since we are still open for discussions here, but feedback on this patch is more than welcome. Simon, I kept the enum instead of a string for the domain mainly because I didn't really get the advantage of using strings for this, since all need is a comparison in the end. From the application perspective, what is the use case of exposing an error domain string? Moreover, I couldn't think of a way of exposing something like WebView.HTTPErrorDomain to QML, being it a constant static string part of the WebView type. =/ Or maybe I misunderstood something here... Nevertheless, this patch modifies the enum ErrorType to ErrorDomain and adds the localized error description string as a parameter of loadFailed. How do you feel about it?
Comment on attachment 119077 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=119077&action=review > Source/WebKit2/UIProcess/API/qt/qquickwebview_p.h:73 > + EngineErrorDomain, Internal?
Comment on attachment 119077 [details] Patch I think this is a step in the right direction. I see that I'm the only one favouring strings, I believe Tor Arne also prefers the enum. Perhaps we should go that route then :)
(In reply to comment #5) > (From update of attachment 119077 [details]) > I think this is a step in the right direction. I see that I'm the only one favouring strings, I believe Tor Arne also prefers the enum. Perhaps we should go that route then :) Ok! :) Is there anything else missing here? (In reply to comment #4) > > Source/WebKit2/UIProcess/API/qt/qquickwebview_p.h:73 > > + EngineErrorDomain, > > Internal? InternalErrorDomain, then? Thanks for the reviews, folks!
(In reply to comment #6) > (In reply to comment #5) > > (From update of attachment 119077 [details] [details]) > > I think this is a step in the right direction. I see that I'm the only one favouring strings, I believe Tor Arne also prefers the enum. Perhaps we should go that route then :) > > Ok! :) > > Is there anything else missing here? I don't think so.
Created attachment 119912 [details] Patch
Comment on attachment 119912 [details] Patch Clearing flags on attachment: 119912 Committed r103310: <http://trac.webkit.org/changeset/103310>
All reviewed patches have been landed. Closing bug.