Tidy up LoadEvent mess

I guess either of the first two options would work.

I don’t know about WebKitGTK, but I just tested QtWebKit and it doesn’t provide a default error page (nor does it provide a way for applications to set one). When loading e.g. "http://invalid/" with QtWebKit, we get two load events: load started, and then load failed.

Whenever we implement a default error page, it would be good to expose an API to allow applications to override it.

Ok, I'll probably just go with disabling it entirely for now then, so we'll behave more like QtWebkit

It turns out that disabling the built-in error page has unintended consequences.

When the initial load fails, the pending navigation entry is removed. When the error page load starts, a new pending entry is created and this gets committed with the URL that caused the error. This means that if we remove the error page then WebView.url reverts to the previously committed URL when a load fails, which is not the expected behaviour (it also explains why the committed event has a different URL).

I'm going to fix this by removing the extra started and succeeded events, and adding LoadEvent.isErrorPage. This means that the new sequence will be:

type=started, url="http://invalid/", isErrorPage=false
type=failed, url="http://invalid/", isErrorPage=false
type=committed, url="http://invalid/", isErrorPage=true

... with the committed event indicating that the webview has been navigated and the old document no longer exists. There are already other cases where we get a committed event without any corresponding started or succeeded events - eg, in-document navigations (where only the URL fragment changes) will do this.

The fix for this does change the behaviour of WebView.loadEvent in a way which would break applications that depend on the broken behaviour. However, as this API was only introduced in 1.3 and the browser isn't using it, I don't think there will be any consequences for doing this. I wonder whether we should backport it to both 1.4 and 1.3 so that we don't end up with people depending on this bug (which would mean that we would have to fix it by introducing yet another new signal, whilst we keep the existing signal broken)?

Yes, I think it would be best to backport the fix to 1.3 (and as a bonus it would allow us to start using it in the browser right away), as well as to 1.4.

The change looks good to me. Shouldn’t we add a Q_REVISION marker to the new isError property?

I did think about that, but as LoadEvent can't be instantiated in QML, it doesn't really suffer the same scoping issues as other API's

Right. However if the fix is backported to 1.3, client applications using the new property should be able to express that they want to use it by importing com.canonical.Oxide 1.3, and know that they won’t get it when importing an earlier version.