cefsharp / CefSharp

.NET (WPF and Windows Forms) bindings for the Chromium Embedded Framework
http://cefsharp.github.io/
Other
9.87k stars 2.92k forks source link

Doc - Clarify LifeSpanHandler.OnBeforePopup return true behaviour #3847

Closed SchreinerK closed 2 years ago

SchreinerK commented 3 years ago

override LifeSpanHandler.OnBeforePopup throws exception

System.Exception: 'returning true cancels popup creation, if you return true newBrowser should be set to null.Previously no exception was thrown in this instance, this exception has been added to reduce the number of support requests from people returning true and setting newBrowser and expecting popups to work.'

From docu:

newBrowser EXPERIMENTAL - A newly created browser that will host the popup. Set to null for default behaviour. To cancel creation of the popup window return true otherwise return false. Source: https://cefsharp.github.io/api/94.4.x/html/M_CefSharp_ILifeSpanHandler_OnBeforePopup.htm

if I set the newBrowser to a new browser und return true I expect that the request will be handled in the specified new browser (tab) and no popop is opened. instead I get an exception "to reduce the number of support requests" What??

Either the docu is wrong or it is a bug because it does not work as described in the docu.

I have also tried to return false, but then a new popop is opened, but this is not desired. If the newBrowser property doesn`t work for this case yet, it should be documented instead than just throwing an exception.

amaitland commented 3 years ago

Please provide a code example.

SchreinerK commented 3 years ago
public bool OnBeforePopup(IWebBrowser browserControl, IBrowser browser, IFrame frame, string targetUrl,
    string targetFrameName, WindowOpenDisposition targetDisposition, bool userGesture,
    IPopupFeatures popupFeatures, IWindowInfo windowInfo, IBrowserSettings browserSettings,
    ref bool noJavascriptAccess, out IWebBrowser newBrowser)
{
    var args = new NewWindowRequestedEventArgs(browserControl, browser, frame, targetUrl, targetFrameName, targetDisposition, userGesture, popupFeatures, windowInfo, browserSettings, noJavascriptAccess);
    NewWindowRequested?.Invoke(this, args);
    noJavascriptAccess = args.NoJavascriptAccess;
    newBrowser = args.NewBrowser;
    return args.Handled;
}

https://github.com/KsWare/KsBrowser/blob/Exception-returning-true-cancels-popup/src/KsBrowser/CefSpecific/MyLifeSpanHandler.cs

properties assigned in new tab

public override async void Initialize(object parameter) {
    switch (parameter) {
        case null:
            throw new ArgumentNullException(nameof(parameter));
        case PrivateNewWindowRequestedEventArgs args: {
            Debug.WriteLine($"InitializeNewPresenter NewWindowRequested");
            if (args.Referrer == null) throw new ArgumentNullException(nameof(PrivateNewWindowRequestedEventArgs.Referrer));

            args.CoreArguments.NewBrowser = ChromiumWebBrowser;
            args.CoreArguments.Handled = true;
            break;
        }
        case Uri uri:
            Debug.WriteLine($"InitializeNewPresenter {uri}");
            await NavigateToUriAsync(uri);
            break;
        default:
            throw new ArgumentOutOfRangeException(nameof(parameter), $"Type not supported. Type:'{parameter.GetType().FullName}'");
    }
}

https://github.com/KsWare/KsBrowser/blob/Exception-returning-true-cancels-popup/src/KsBrowser/WebContentPresenter/(Cef)/CefSharpControllerVM.cs

amaitland commented 3 years ago

The document is roughly based on the upstream header https://github.com/chromiumembedded/cef/blob/ba46b8c53ed0f46cec82a1aebaec173784deb6e4/include/cef_life_span_handler.h#L82

if I set the newBrowser to a new browser und return true I expect that the request will be handled in the specified new browser (tab) and no popop is opened.

Returning true cancels the popup. The request is cancelled and no further action occurs.

Either the docu is wrong or it is a bug because it does not work as described in the docu.

Your are welcome to submit a PR clarifying the behaviour.

Assigning the existing ChromiumWebBrowser instance to newBrowser is not currently supported, hence the exception.

Improvements to the document are welcomed, labeling as up for grabs.

SchreinerK commented 3 years ago

Assigning the existing ChromiumWebBrowser instance to newBrowser is not currently supported, hence the exception.

newBrowser is not the exsting browser (not the same instance as in browserControl) but a newly created ChromiumWebBrowser on a newly created TabItem. With "newly created" I mean control is created and attached to UI tree but not yet navigated to any url.

Your are welcome to submit a PR clarifying the behaviour.

Once I figure out how it works correctly, I can do this

What conditions must be met to use newBrowser property?

amaitland commented 3 years ago

newBrowser is not the exsting browser (not the same instance as in browserControl)

If you return true the request is cancelled, newBrowser needs to be null. If you return true and newBrowser is not null then you get an exception. The exception exists because I was getting a lot of support requests from users setting newBrowser and returning true and wondering why nothing happened.

You are welcome to improve the exception message.

What conditions must be met to use newBrowser property?

See https://github.com/cefsharp/CefSharp/blob/cefsharp/94/CefSharp.Core.Runtime/Internals/ClientAdapter.cpp#L143

There is currently only a WPF example of using the experimental feature at https://github.com/cefsharp/CefSharp/blob/cefsharp/94/CefSharp.Wpf.Example/Handlers/ExperimentalLifespanHandler.cs#L14

It's not something I've tested in a long time, so may not work.

If you are using WinForms the recommended approach for hosting the popup as a tab is https://github.com/cefsharp/CefSharp/pull/3529

amaitland commented 3 years ago

There is also some outdated documentation at https://github.com/cefsharp/CefSharp/wiki/General-Usage#popups that needs a refresh .

lsim commented 2 years ago

I was reading through this thread after I got stumped on the same problem.

What is the intended use of the newBrowser out-argument?

API-wise OnBeforePopup reads like a hook for providing one's own window with a ChromiumBrowser instance in it. Great!

But if I return true to avoid the default popup window, I am not allowed to provide my own popup instance (newBrowser must be null). And if I return false to be allowed to provide my own, CefSharp pops up its own window in addition to the one I have popped up.

What I have been doing so far is to set newBrowser to null, return true and show my own popup with a ChromiumBrowser instance in it. That works except when the javascript that called window.open() wants to interact with the child window after the fact. This fails since a reference to the child instance was not passed back to the parent instance.

Hope someone can help me make sense of this.

\<edit> I dove into the links posted above and tried to replicate the necessary from the wpf example. This helped somewhat, in that I got a little closer to the goal. When the parent window tries to write to the body of the popup, however, a native exception in Cef tears down my entire application. Giving up for now. \</edit>

amaitland commented 2 years ago

What is the intended use of the newBrowser out-argument?

At the low level is allows for assigning the CefClient instance associated with the ChromiumWebBrowser to the CefClient param of the CefLifeSpanHandler::OnBeforeBrowser method. Doing so means the handlers (lifespanhandler, displayhandler, etc) will be called for the CefClient instance associated with the newBrowser instance, rather than the parent ChromiumWebBrowser which is the default.

But if I return true to avoid the default popup window, I am not allowed to provide my own popup instance (newBrowser must be null).

By default the popup (browser) is opened in a new native window. If you return true then popup (browser) creation is cancelled. return true means don't create the popup (browser) at all.

And if I return false to be allowed to provide my own, CefSharp pops up its own window in addition to the one I have popped up.

To allow popup (browser) creation you must return false. There is still WinForms or WPF specific code that's required to

What I have been doing so far is to set newBrowser to null, return true and show my own popup with a ChromiumBrowser instance in it. That works except when the javascript that called window.open() wants to interact with the child window after the fact. This fails since a reference to the child instance was not passed back to the parent instance.

That's expected as the popup (browser) creation was cancelled. So there's no parent/child relationship.

I dove into the links posted above and tried to replicate the necessary from the wpf example. This helped somewhat, in that I got a little closer to the goal. When the parent window tries to write to the body of the popup, however, a native exception in Cef tears down my entire application. Giving up for now.

What does your code look like?

lsim commented 2 years ago

Thanks for getting back to me.

Looks like I'll need to look into the .SetAsChild() invocation, you mention. It doesn't seem to be featured in the example

I've got this:

    public event Func<string, (IWebBrowser, Window, bool)> OnPopupRequested;

    public bool OnBeforePopup(IWebBrowser chromiumWebBrowser, IBrowser browser, IFrame frame, string targetUrl,
      string targetFrameName, WindowOpenDisposition targetDisposition, bool userGesture, IPopupFeatures popupFeatures,
      IWindowInfo windowInfo, IBrowserSettings browserSettings, ref bool noJavascriptAccess, out IWebBrowser newBrowser)
    {
      if (!(chromiumWebBrowser is ChromiumWebBrowser parentBrowser)) 
        throw new Exception("Got unexpected parent frame type" + chromiumWebBrowser?.GetType().Name);

      var (popupBrowser, doDefault) = parentBrowser.Dispatcher.Invoke(() =>
      {
        var (thePopupBrowser, popupWindow, theDoDefault) = OnPopupRequested?.Invoke(targetUrl) ?? (null, null, false);
        if (thePopupBrowser != null)
        {
          thePopupBrowser.SetAsPopup();

          var windowInteropHelper = new WindowInteropHelper(popupWindow);
          var handle = windowInteropHelper.EnsureHandle();
          windowInfo.SetAsWindowless(handle);

          popupWindow.Closed += (sender, args) => thePopupBrowser.Dispose();
        }

        return (thePopupBrowser, theDoDefault);
      });

      newBrowser = popupBrowser;

      // CEF api: return true to prevent a popup window from being created, false otherwise
      return !doDefault;
    }

For the case where a window is returned from the event handler, doDefault is true, which causes OnBeforePopup to return false.

What the above does successfully, is to prevent the default native popup window from being created. The parent window javascript is able to eg set the title of the popup via the reference returned from window.open(). It's only when writing the body element that things blow up. This is my test code (works in FF/chrome):

    document.getElementById('popupBn').addEventListener('click', function(e) {
      var w = window.open('about:blank', 'fooWindowName', "menubar=0,resizable=1,location=0,status=0,scrollbars=1,width=640,height=480");
      try {
        w.darkTheme = false; // This succeeds
        w.document.write("<title>Foo Title</title>"); // This succeeds
        w.document.write("<body>This is the new body</body>"); // This fails
        w.document.close();
        console.log('popup written', w.document.body.outerHTML)
      } catch (e) {
        console.log('popup error', e);
      }
    });
amaitland commented 2 years ago

Looks like I'll need to look into the .SetAsChild() invocation, you mention

This would be for WinForms only as stated above. WPF uses SetAsWindowless.

It's only when writing the body element that things blow up. This is my test code (works in FF/chrome):

Does the same example code work using the default popup opened in a new window?

Are there errors in the cef log file?

lsim commented 2 years ago

This would be for WinForms only as stated above. WPF uses SetAsWindowless.

My bad - missed that on my first read through.

Does the same example code work using the default popup opened in a new window?

Yes it does

Are there errors in the cef log file?

debug.log contains just the console log output from the page - no errors. I tried setting it to verbose logging though and found that the last log line from the test code above does in fact get run, which suggests I may have been telling half truthes about exactly when the error happens.

What I more specifically observed is that the problem goes away if I comment out the line

 w.document.write("<body>This is the new body</body>"); // This fails

With the body modifying line, the dialog dies a short moment after the dialog has appeared. The This is the new body text is never rendered, so perhaps the crash occurs as it attempts to render that change?

The popup is loaded with the url about:blank. Not sure if that is somehow significant.

amaitland commented 2 years ago

The popup is loaded with the url about:blank

about:blank is a special case where no render process is spawned by default, best to avoid these days.

I can reproduce the problem, exception appears to be coming from libcef.dll, so likely a CEF issue when hosting the popup with OSR (likely this has never been tested).

Loading an empty html page on the same domain and writing to the document when the DOM has finished loading appears to be working for a basic test. So you can try work around the issue.

document.getElementById('openMyWindow').addEventListener('click', function (e)
{
    myWindow = window.open('/HelloWorld.html', 'Popup WindowName', "menubar=0,resizable=1,location=0,status=0,scrollbars=1,width=640,height=480");

    try
    {
        myWindow.darkTheme = false;

        myWindow.addEventListener('DOMContentLoaded', (event) =>
        {
            myWindow.document.write("<title>Foo Title</title>");
            myWindow.document.write("<body>This is the new body</body>");
            myWindow.document.close();

            //myWindow.document.title = "Foo Title";
            //myWindow.document.body.innerHTML += "<br/>Appended to document body";

            console.log('Popup body and title updated:', myWindow.document.body.outerHTML);
        });                
    }
    catch (e)
    {
        console.log('popup error', e);
    }
});
lsim commented 2 years ago

Thanks for looking into it!

As I read your comment, the workaround is to use an empty html page on the same domain in stead of the about:blank approach. Unfortunately that doesn't help in this case since I am working on an automation solution. The website loaded into the cef component is the one being automated and so must be expected to contain pretty much any markup/scripting that Chrome supports.

Prior to this specific customer complaint, I hadn't even heard of this approach of basing a popup on about:blank and populating it by scripting :)

Looks like this needs to go on our 'known issues' pile for now

Thanks again for your time

amaitland commented 2 years ago

If this is something you'd like to see fixed then you'll need to contribute your time to debugging the issue. You can enable crash reporting, get a dump, load libcef.dll.pdb and analyze the dump to get a call stack.

https://github.com/cefsharp/CefSharp/wiki/Trouble-Shooting#unmanaged-crashes-when-the-process-dies

CEF has it's own issue tracker on bitbucket at https://bitbucket.org/chromiumembedded/cef/issues?status=new&status=open

amaitland commented 2 years ago

There is also some outdated documentation at https://github.com/cefsharp/CefSharp/wiki/General-Usage#popups that needs a refresh .

Done in https://github.com/cefsharp/CefSharp/wiki/General-Usage/_compare/5c438c4bd0adcb334877dc4f30ced73635ff0848...6ddaaf9ea94af702a9e9c28662f5a44aba4c838c

The doc has been updated in commit https://github.com/cefsharp/CefSharp/commit/299f63e64fd913d6be4fed57cb37c8335e08dc30

Anyone wants to try making further clarifications then a PR would be most welcome.