Navigator: bug and enhancement

I’m glad to see that the Navigator component has been improved and debugged a lot in Beta1 compared with Alpha3. There’s still one bug left in the compontent. At least I assume that this is not intended as a feature :wink: . In the current implementation, you are forced to use URI fragments that start with a shebang #!. If you don’t use that scheme and your fragments only begin with the hash sign, Navigator will not navigate to your views. The culprit here is method Navigator.UriFragmentManager#getState():


 @Override
public String getState() {
	String fragment = page.getFragment();
	if (fragment.startsWith("!")) {
		return page.getFragment().substring(1);
	} else {
		return "";
	}
}

If the current fragment doesn’t start with the exclamation mark, getState() returns the empty String which is then used as the navigation target. The method should instead read


 @Override
public String getState() {
	String fragment = page.getFragment();
	if (fragment.startsWith("!")) {
		return page.getFragment().substring(1);
	} else {
		// return the whole fragment
		return page.getFragment(); 
	}
}

As an enhancement to Navigator consider the following suggestion. Method Navigator#navigateTo(String) contains a TODO reading “TODO if no view is found, what to do?”. In the current implementation, Navigator simply does nothing if a URI fragment is visited for which no view has been registered. That’s fine for a default implementation. It would be nice, though, if it was possible to register a default view to which the Navigator navigates by default when no other view could be found. Implementers could then define some Page-Not-Found or Catch-All-Unhandled-Fragments view. It would be even nicer if it was possible to send back a 404 error code. If that is possible at all in this context where no “real” HTTP request has been sent to the server. But I guess, this could be simulated by that default Page-Not-Found view which could simply redirect to a real 404 error page.

What do the Vaadin people think? Is that worth to create two corresponding tickets in Trac?

Here’s a
ticket
where it has been decided that the separator should be #!. The decision had to do with browser compatibility.

Other than that, I haven’t had time to familiarize with the beta1 version of the Navigator, so I can’t express my opinions just yet.

This is actually more for compatibility with robots etc. than browsers. If you want to keep supporting/using just “#”, you can use your own implementation of NavigationStateManager - e.g. a subclass of Navigator.UriFragmentManager.
Perhaps we should also consider extending the default functionality as you proposed - an enhancement request would help us not forget this.

As for missing views, Johannes D is currently considering options for handling those.

That the default UriFragmentManager only support hashbang fragments was a deliberate design decision from my part, although not something I have a strong opinion on. The point was to keep URIs somewhat consistent and to offer some support for regular “anchor” fragments (aka #foo should scroll to element id=“foo”). As Henri said, for a more general case you should write your own NavigationStateManager. UriFragmentManager is currently a bit hard to extend, though.

Handling missing views will be added in beta 2; the mechanism didn’t quite make it into beta 1. Ticket
#9060
tracks this issue.

Thanks for your feedback, guys. That sounds promising. Only supporting hashbang fragments is ok for me. I just found it a bit confusing that Navigator.UriFragmentManager#getState() simply returns an empty String for normal URI fragments. If that was a deliberate decision then it’s ok the way it is.