Links with stream resource are broken in CCDM

[denis-anisimov]

Use com.vaadin.flow.uitest.ui.StreamResourceView test.

It sets a link (a element) with a dynamic resource via StreamResource:

 StreamResource resource = new StreamResource("filename",
                () -> new ByteArrayInputStream(
                        "foo".getBytes(StandardCharsets.UTF_8)));
        Anchor download = new Anchor("", "Download file");
        download.setHref(resource);
        download.setId("link");
        add(download);

Open the http://localhost:8888/view/com.vaadin.flow.uitest.ui.StreamResourceView page.
There will be a link.
Click on it.

Result: there is an attempt to apply a routing for the URI. But there is no such route. Navigation can't be done and there is a routing error.

Expected: the file is downloaded.

In fact the file can be downloaded : you may copy a link to the clipboard and open it in another tab.
Then everything works fine.

So apparently routing is applied when it should not be applied. That worked fine without CCDM.
Most likely broken because of CCDM.

[platosha]

You can mark such links with a download attribute to make Vaadin Router ignore them. Also, router-ignore.

Will update the docs.

[platosha]

Update after f2f between @Legioth and CCDM team: we need to think more and have a design decision here. Right now there is a mismatch between defaults in Vaadin Router and Flow:

• In Flow, before 3.0, client only handles links marked with router-link attribute
• Vaadin Router handles all clicks except marked with download or router-ignore attributes

Options:

1. Keeping implementation as is. This means, that the default behaviour changes in Flow, starting from 3.0 all the links will be handled as client-side navigation in Vaadin Router.

Links that are not for client-side navigation must opt-out explicitly.

Pros: Client-side views with client-side navigation are simple, as they can just use standard links.

Cons: Breaking for some existing links in Flow:

- Anchor class
- Download links
- Links that target same origin but are handled outside Flow, e. g., other servlets, external login forms from Spring Security, and such

2. Keep routing implemented as is, but adjust the Anchor class to opt-out from client-side navigation by default.

Pros: Client-side views are simple too. Anchor usage is not affected. Downloads using Anchor work by default.

Cons: Still breaking for links in Flow that are not Anchor and not client-side.

3. Keep Flow 2.0 default and change Vaadin Router accordingly. Links in client-side views have to specify router-link to not cause a page reload.

Pros: Not breaking anything for Flow.

Cons: Breaking change in Vaadin Router. Slightly harder to build links in client-side views, requires a router-link attribute on top of standard API. Maybe introducing a client-side template helper for links could help with that, e. g., Lit directive?

4. Keep Flow 2.0 default, and instead of changing Vaadin Router, have a workaround implemented as a custom Vaadin Router click trigger in Flow.ts

Pros: Not breaking anything.

Cons: Sample as above, except does not require a breaking change in Vaadin Router. Some additional bundle and maintenance costs for the workaround.

[Legioth]

Option 2 has the unexpected side effect that <a href=foo>foo</a> in a HTML template would work differently than new Anchor("foo", "foo") from Java in the same project. This gets even more confusing combined with @Id since that changes a template element to suddenly behave like a Java component.

We can still consider giving special treatment to Anchor instances configured to use a StreamResource rather than a String href since that variant doesn't have a corresponding HTML representation.

Another potential approach would be to go with option 1, but also document how to reconfigure the client-side router to work in the old way in order to ease migration. In any case, any of the alternatives would also need to be clearly documented.

[Artur-]

As a user, I would expect:

1. Not to write anything else than <a href="my-view> to create a link to a view
2. Not to have to distinguish between server side and client side views when creating links
3. Not to even hear about internal concepts such as "client side router" and "server side router". There is only one visible routing feature
4. That <a href="foo"> and new Anchor("foo") is the same thing

[platosha]

Concluded in F2F:

• The preferred default is to not have page reloads with basic <a href="my-view"> links, in other words, the Vaadin Router’s current default is fine
  • The majority of links in apps are expected to navigate in the app, and not to download or navigate outside
  • Developers don’t have to figure it out and declare anything extra to make them work nicely
  • External links (i. e., targeting different origin) are already excluded
• Existing RouterLink usage works with the new default
• The Anchor should by default continue to create same pure <a href=""> links without anything extra
• This is breaking existing Anchor usage in cases when links are intended for browser handling. This should be communicated in release notes with an opt-out workaround.
• We should fix the case of linking to StreamResource, such a link should opt-out from client-side handling using the router-ignore attribute
• In addition, we could improve the not found error view to mention the opt-out workaround

[platosha]

As far as I understood, the fix of linking to StreamResource and the error view improvement are not blocking the stable.

[Artur-]

Why was this closed without the issue being fixed?

[haijian-vaadin]

Let's reopen it. I was dragging the related PR to the Closed column, the ticket moved along with the PR.

[alvarezguille]

The fix for this issue broke the behavior when router-ignore attribute is added before calling setHref(String)

		a.getElement().setAttribute("router-ignore", true);
		a.setHref(contextPath + "/logout");

Like it's done in full stack starter

[Legioth]

We shouldn't remove that attribute if it wasn't added by us.

It might be easiest to just never remove it since it's quite rare to on-the-fly change between a stream resource and a regular href.

[haijian-vaadin]

ok, I made a new ticket #7804 for the new issue and closing this one.

[knoobie]

Why is this mentioned as limitation in the V23 release? Isn't that fixed since a long time?