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

[feat] navigation handler hook for plugins #7306

Closed
jhutchins opened this issue Jun 26, 2023 · 8 comments
Closed

[feat] navigation handler hook for plugins #7306

jhutchins opened this issue Jun 26, 2023 · 8 comments

Comments

@jhutchins
Copy link
Contributor

jhutchins commented Jun 26, 2023

Describe the problem

I have some use cases where it would be useful for a plugin to be able to add a navigation handler for it own usage.

Describe the solution you'd like

I would like a with_navigation_handler function or similar added to the Plugin interface.

Alternatives considered

Something similar could probably be achieved by adding an event hook into wry that created a WindowEvent when navigation started and then using the pre-existing on_event method.

The on page load kind of works, but it only triggers after a page starts loading instead of preventing navigation.

Additional context

No response

@amrbashir
Copy link
Member

The main problem with this is when multiple navigation handlers are registered, how would you determine which one have priority over the other one?

@jhutchins
Copy link
Contributor Author

You would have one wrapper handler that runs all the individually registered handlers and returns their ANDed results.

fn navigation_handler(&self, url: String) -> bool {
    let mut result = true;
    for handler in self.handlers {
        result = result && handler(url);
    }
    result
}

@FabianLars
Copy link
Member

Good idea imo, but what if I would like to overwrite a plugin's behavior in my own app? Or should this be covered by the plugin authors offering an API for this? 🤔

@amrbashir
Copy link
Member

You would have one wrapper handler that runs all the individually registered handlers and returns their ANDed results.

fn navigation_handler(&self, url: String) -> bool {
    let mut result = true;
    for handler in self.handlers {
        result = result && handler(url);
    }
    result
}

Doesn't sound like a good idea to me, why run subsequent navigation handlers if a previous one has disallowed it anyways? Maybe some handlers might have logic that depends on whether they allowed the navigation or not and with the proposed solution, it can't actually know for sure if the navigation was allowed or not.

@jhutchins
Copy link
Contributor Author

@FabianLars I would think that in that case that you want to override the plugin's behavior that either it would be expected to have an API in the plugin for that, or you would simple need to not use the plugin.

@amrbashir the reason for running all the handlers would be for side effects of the handlers. My primary use case for this if for login callbacks for an application that interacts with audible, but I'm sure there could be any number of useful side effects. Also your statement is only partially true, if a plugin disallows navigation it will know for certain that the navigation will not occur, it's only that it can't be sure that navigation will actually continue just because it allows it. This is a constraint that plugin authors and users can be aware of and design for. It's not like this is actually more limiting than not being able to control this at all, which is the current status quo.

@amrbashir
Copy link
Member

amrbashir commented Jul 4, 2023

Yeah, don't get me wrong I am not trying to block this, I am just thinking out loud to make sure we cover all the cases.

how about breaking out of the loop when the first block is encountered? then you'd have to decide which handler has priority over this, and my idea is that it would be up to the user using the plugin, the handle of the first plugin registered is executed first then the one after that and so on...

fn navigation_handler(&self, url: String) -> bool {
  if !window_defined_handler(url) {
    return false;
  }

  if !tauri_app_builder_defined_handler(url) {
    return false;
  }

  for handler in plugin_handlers {
      if !handler(url) {
        return false;
      }
  }
  
  true
}

so users can control the execution behavior like this

tauri::Builder::default()
	.plugin(plugin_a::init()) // plugin A will have its navigation handler run before plugin B
	.plugin(plugin_b::init())
	.run()
	.expect("error msg")

and if navigation was stopped by a handler, then remaining handlers won't need to run because a navigation has been stopped anyways so they won't need to run any side effects

@jhutchins
Copy link
Contributor Author

jhutchins commented Jul 4, 2023

This would fine for what I wanted too. As far I can tell the strength of this approach is that potentially less code needs to be run per navigation (because of short circuiting). The weakness would be that it doesn't allow the usage of multiple plugins that trigger callbacks on navigation to the same url where are least one cancels the navigation, which I'm not convinced would be a real problem in practice.

In short, I would be fine doing it this way.

@lucasfernog
Copy link
Member

It feels kinda weird that a plugin can interfere with such an app-specific (IMO) event which is the navigation. But if all of you see the value I'll take it.

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

4 participants