Bug 29644

Summary: [GTK] Add WEBKIT_LOAD_ERROR status
Product: WebKit Reporter: Xan Lopez <xan.lopez>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: jmalonzo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
loaderror.diff jmalonzo: review+, xan.lopez: commit-queue-

Description Xan Lopez 2009-09-22 09:09:20 PDT
Page loads can fail to be completed and reach WEBKIT_LOAD_FINISHED status (user cancels the load, non-existent URI, etc). At the moment the only way to figure out this is to connect to both notify::load-status and WebKitWebView::load-error, which is far from ideal IMHO. This patch adds a new load-status enum, WEBKIT_LOAD_ERROR, which indicates that some error happened and that the load was stopped. In this way it's still possible to handle the whole flow of the load by only connecting to a single signal.

Tests included.
Comment 1 Xan Lopez 2009-09-22 09:15:04 PDT
Created attachment 39926 [details]
loaderror.diff
Comment 2 Gustavo Noronha (kov) 2009-09-22 10:05:47 PDT
Comment on attachment 39926 [details]
loaderror.diff

I like the idea. You have my half r+. I'll take this into consideration when re-submitting sub-resource load tracking.
Comment 3 Jan Alonzo 2009-09-22 12:09:27 PDT
(In reply to comment #1)
> Created an attachment (id=39926) [details]
> loaderror.diff

Does this only appply to committed load errors?
Comment 4 Xan Lopez 2009-09-22 12:27:23 PDT
(In reply to comment #3)
> (In reply to comment #1)
> > Created an attachment (id=39926) [details] [details]
> > loaderror.diff
> 
> Does this only appply to committed load errors?

Not really, when a URL does not exist we'll go from provisional to error directly. Why?

BTW, I'm thinking that WEBKIT_LOAD_FAIL might be a better name than WEBKIT_LOAD_ERROR, since the dispatch is called didFailLoad. Opinions? :)
Comment 5 Xan Lopez 2009-09-22 12:30:28 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #1)
> > > Created an attachment (id=39926) [details] [details] [details]
> > > loaderror.diff
> > 
> > Does this only appply to committed load errors?
> 
> Not really, when a URL does not exist we'll go from provisional to error
> directly. Why?
> 
> BTW, I'm thinking that WEBKIT_LOAD_FAIL might be a better name than
> WEBKIT_LOAD_ERROR, since the dispatch is called didFailLoad. Opinions? :)

On the other hand the existing signal is called 'load-error', hmmm... Naming things sucks.
Comment 6 Jan Alonzo 2009-09-22 12:44:35 PDT
Comment on attachment 39926 [details]
loaderror.diff

(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > (In reply to comment #1)
> > > > Created an attachment (id=39926) [details] [details] [details] [details]
> > > > loaderror.diff
> > > 
> > > Does this only appply to committed load errors?
> > 
> > Not really, when a URL does not exist we'll go from provisional to error
> > directly. Why?
> > 
> > BTW, I'm thinking that WEBKIT_LOAD_FAIL might be a better name than
> > WEBKIT_LOAD_ERROR, since the dispatch is called didFailLoad. Opinions? :)

I prefer LOAD_FAIL.

> On the other hand the existing signal is called 'load-error', hmmm... Naming
> things sucks.

Can we break API? Either way, r=me.
Comment 7 Xan Lopez 2009-09-24 05:55:11 PDT
Landed in r48719 renaming the status to LOAD_FAILED as discussed on IRC.