Oxide

Refine WebView.navigationRequested and WebView.newViewRequested

Bug #1300891 reported by Chris Coulson
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Oxide
Fix Released
Medium
Unassigned

Bug Description

I initially thought that all top-level navigations would result in WebView.navigationRequested firing, and that this could be used to block all navigations whether they are going to happen in the current webview or not (accepting a navigation with disposition != CurrentTab would then result in WebView.newViewRequest firing, and the application creating a new WebView). Using WebView.newViewRequest to block a navigation in a new view doesn't really work correctly at the moment due to bug 1300884.

However, there are some exceptions which make the current API a bit confusing:

- window.open() only results in WebView.newViewRequested (although I knew about this one)
- Clicking on an anchor with target="_blank" only results in WebView.newViewRequested
- Clicking with modifiers on an anchor that has no target results in both WebView.navigationRequested and WebView.newViewRequested.
- Subframe navigations that have a disposition of CurrentTab don't result in any signal, but this one is expected.
- Browser-initiated navigations don't result in either signal, but this is deliberate too.

I think this should probably be cleaned up by:
- Removing NavigationRequest.disposition and only dispatching WebView.navigationRequest for top-level renderer-intiated navigations for the current view.
- Dispatch only WebView.newViewRequested for all navigations that are not destined for the current WebView, which requires fixing bug 1300884 so that this can be used to reject requests.

Chris Coulson (chrisccoulson)
Changed in oxide:
importance: Undecided → High
status: New → Confirmed
status: Confirmed → Triaged
Revision history for this message
Chris Coulson (chrisccoulson) wrote :

Actually, thinking about this a bit more, this is probably ok - it would allow extending NavigationRequest later on to turn a same-window navigation in to a new-window navigation (and vice-versa) by adding some more values to the Action enum

Changed in oxide:
importance: High → Medium
Revision history for this message
Chris Coulson (chrisccoulson) wrote :

Ok, so I've had more of a thought about this API today - particularly wrt bug 1300884. The main issue with newViewRequested is that it's being used for what should be 2 separate tasks:

- Creating a new WebView
- Displaying the new WebView when it's ready.

It's currently called from 2 different code paths, with each path dispatching the signal at a different point (window.open() results in the signal being emitted when the renderer wants to show the newly created view. Clicking on a link results in it being called before anything is created in Chromium).

I think newViewRequested should be split in to 2 signals (newViewRequested and presentNewView, or something like that). The position property should be removed from NewViewRequest, and instead provided via the presentNewView signal. The way this would work is:

- On WebView.newViewRequested, the application creates a new WebView using the disposition as a hint for the type of view, but doesn't display it
- On WebView.presentNewView (or whatever it should be called), the application displays the new WebView (or not, if its a new background tab) using the provided position rect as a hint for the size and location of the new view if it's in a new window.

How does that sound? There is a precedent for this already - the WebKitGTK API splits this in to 2 signals as well

Revision history for this message
Chris Coulson (chrisccoulson) wrote :

I ended up fixing this without adding a third signal in the end.

Instead, it now dispatches WebView.navigationRequested for *all* requests to open a new window, regardless of the origin of these. This is dispatched before any resources are created and should be used if the application wants to block a window from opening.

WebView.newViewRequested is now always dispatched when the underlying WebContents is ready to display. If an application implements this, it is expected that it creates a new WebView. It shouldn't be used for policy decisions (use navigationRequested for that), so I've removed NewViewRequest.url and NewViewRequest.userGesture to discourage this. I'll probably make it output a console warning if an application implements this but doesn't create a WebView.

Changed in oxide:
status: Triaged → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.