Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using LoadHtml/LoadUrl for displaying errors breaks back button #2985

Closed
mitchcapper opened this issue Dec 9, 2019 · 6 comments
Closed

Using LoadHtml/LoadUrl for displaying errors breaks back button #2985

mitchcapper opened this issue Dec 9, 2019 · 6 comments
Milestone

Comments

@mitchcapper
Copy link
Contributor

After #2921 the best way to handle errors seemed to be using LoadHtml/LoadUrl the problem of this is that it creates an additional history entry that prevents the back button from properly functioning. The back button takes you to the same error site that re-triggers the error. We could work around this by not showing the error the second time but that would still require hitting the back button twice.

A temporary solution is to inject JS to call the history.go(-2) if on an error page but isn't great.

Per @amaitland suggestion created an upstream discussion thread about suggested best practice with some suggested options:
https://www.magpcss.org/ceforum/viewtopic.php?f=10&t=17250

@amaitland amaitland added the upstream These issues require fixing in the Chromium Embedded Framework(CEF) or Chromium. label Dec 9, 2019
@amaitland amaitland changed the title Using LoadHtml/LoadUrl for displaying errors breaks back button (upstream) Using LoadHtml/LoadUrl for displaying errors breaks back button Dec 10, 2019
@mitchcapper
Copy link
Contributor Author

OK! So I missed the initial reply but he pointed towards some code that seems like the correct way to do this, the InterstitialPageDelegate.

I found a decent example of this in another project for showing an ssl error page:
https://github.com/crosswalk-project/crosswalk/pull/2985/files

We would need to add some low level binders for the InterstitialPageDelegate but I don't think that would be too difficult. I have a bit of an issue at work currently but once I get a bit of free time will go about implementing. I don't think this is a rush, but it certainly seems like a nicer way to do proper intermediate pages.

@amaitland
Copy link
Member

Using DevTools it's possible to navigate to a specific history entry.

Haven't tested yet, will when I get s change. Still a workaround, might be a slightly nicer one.

@amaitland
Copy link
Member

I think there's a better workaround for displaying errors, initial testing looks promising. Via DevTools you can use Page.SetDocumentContent to display the error page without having to hack around with the back/forward history.

chromiumWebBrowser.LoadError += OnBrowserLoadError;

private async void OnBrowserLoadError(object sender, LoadErrorEventArgs e)
{
	//Actions that trigger a download will raise an aborted error.
	//Aborted is generally safe to ignore
	if (e.ErrorCode == CefErrorCode.Aborted)
	{
		return;
	}

	//using LoadHtml/LoadUrl creates an additional history entry that
	//prevents the back button from working correctly.
	//Use Devools Page.SetDocumentContent to change the content
	using (var client = browser.GetDevToolsClient())
	{
		var response = await client.Page.GetFrameTreeAsync();
		var frames = response.FrameTree;
		var mainFrame = frames.Frame;

		var errorHtml = string.Format("<html><body><h2>Failed to load URL {0} with error {1} ({2}).</h2></body></html>",
									  e.FailedUrl, e.ErrorText, e.ErrorCode);

		_ = client.Page.SetDocumentContentAsync(mainFrame.Id, errorHtml);

		//AddressChanged isn't called for failed Urls so we need to manually update the Url TextBox
		this.InvokeOnUiThreadIfRequired(() => urlTextBox.Text = e.FailedUrl);
	}
}

The WinForms MinimalExample was updated in cefsharp/CefSharp.MinimalExample@3cc3e64 for testing purposes.

@amaitland amaitland removed the upstream These issues require fixing in the Chromium Embedded Framework(CEF) or Chromium. label Jan 5, 2022
@amaitland amaitland added this to the 97.0.x milestone Jan 5, 2022
amaitland added a commit that referenced this issue Jan 5, 2022
- Updated example error handlers to use SetMainFrameDocumentContentAsync for displaying error messages
  so back/forward navigation works correctly.
- DevToolsExtensions updated to use IChromiumWebBrowserBase instead of IWebBrowser so they're usable
  for WinForms hosted popups

Issue #2985
@amaitland
Copy link
Member

Two new SetMainFrameDocumentContentAsync extension methods have been added to simplify setting the main frame content. Important to note that LoadError won't trigger a Address Changed notification, if you have an address bar then you'll need to manually update it with the FailedUrl.

private void OnLoadError(object sender, LoadErrorEventArgs args)
{
	//Aborted is generally safe to ignore
	//Actions like starting a download will trigger an Aborted error
	//which doesn't require any user action.
	if(args.ErrorCode == CefErrorCode.Aborted)
	{
		return;
	}

	var errorHtml = string.Format("<html><body><h2>Failed to load URL {0} with error {1} ({2}).</h2></body></html>",
									  args.FailedUrl, args.ErrorText, args.ErrorCode);

	_ = args.Browser.SetMainFrameDocumentContentAsync(errorHtml);

	//AddressChanged isn't called for failed Urls so we need to manually update the Url TextBox
	this.InvokeOnUiThreadIfRequired(() => urlTextBox.Text = args.FailedUrl);
}

The example projects have been updated.

@amaitland
Copy link
Member

The new extension methods are available in version 97.1.11.

@amaitland
Copy link
Member

CEF will hopefully be adding a function for displaying error pages.

https://bitbucket.org/chromiumembedded/cef/issues/3265/alloy-add-better-support-for-error-pages

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants