search

neos / neos-development-collection

The unified repository containing the Neos core packages, used for Neos development.
https://www.neos.io/
GNU General Public License v3.0
260 stars 221 forks source link

Links to an assetProxy from within content are not evaluated #2356

Open daniellienert opened 5 years ago

daniellienert commented 5 years ago

Description

On the demo site, link to an asset proxy.

Expected behavior

In the frontend, I get a link to the asset proxy.

Actual behavior

I get an assetproxy:// link

Affected Versions

Neos: dev-master

UI:dev-master

daniellienert commented 5 years ago

Video: https://neos-project.slack.com/archives/C0U0KEGDQ/p1548330897281800?thread_ts=1548330878.281600&cid=C0U0KEGDQ

dimaip commented 5 years ago

Not a UI issue, transferring here.

aertmann commented 5 years ago

Can confirm with latest Neos 4.2.5 and Neos UI 3.0.1

dimaip commented 5 years ago

Probably needs to be tackled here, right? https://github.com/neos/neos-development-collection/blob/7bf184dfcda55662f6cb73687eba29848a047705/Neos.Neos/Classes/Fusion/ConvertUrisImplementation.php#L119

aertmann commented 5 years ago

Also here: https://github.com/neos/neos-development-collection/blob/master/Neos.Neos/Classes/Service/LinkingService.php#L68 https://github.com/neos/neos-development-collection/blob/master/Neos.Neos/Classes/Service/LinkingService.php#L206

Potentially also here: https://github.com/neos/neos-development-collection/blob/master/Neos.Neos/Classes/Domain/Service/NodeShortcutResolver.php#L59

aertmann commented 5 years ago

Just wondering why this issue appears now and not before, nothing changed from 4.2.4 to 4.2.5 to cause it as far as I can see. So I'm still suspecting the new UI.

aertmann commented 5 years ago

After a bit more investigation I can say that it is indeed the new UI causing issues.

The 1.x branch works as this was reverted https://github.com/neos/neos-ui/commit/6bd1cfffefa6c9ef4ef5b969e7ba6fcd28712948

The assetProxy:// comes from https://github.com/neos/neos-ui/blob/master/packages/neos-ui-backend-connector/src/Endpoints/index.ts#L331

So it's currently broken due to the new UI supporting the assetProxy://, but not all parts of linking does in Neos. Seems like one of those issues caused by different versioning schemes, but even the master of Neos doesn't support it and there's no pending pull request adding support for it.

Any suggestions how to proceed? I'd say we should fix it as a bugfix as it can easily be done in a backwards compatible way.

dimaip commented 5 years ago

It was reverted for Neos 3.x and should be there in Neos 4.x. I guess it was just not fully implemented yet in the core, by oversight. @robertlemke or @kitsunet could shed some light.

daniellienert commented 5 years ago

We definitely need to fix that in a bug fix version, as all in-content-links generated by the 3.0 of the UI to assets are broken.

dimaip commented 5 years ago

Dear @robertlemke, I reverted https://github.com/neos/neos-ui/pull/2161 for now, as it totally breaks even linking to local assets (they all end up with assetProxy). Let's bring it back in when it's fully supported and tested.

kitsunet commented 5 years ago

Good catch, you are probably right that it was never solved for inline links. Maybe a chance to finally make the backend side configurable (eg. custom schemes to link to things)

kitsunet commented 5 years ago

Hi all, see https://github.com/neos/neos-ui/pull/2385 as a fix for that. This adds the asset source support again, but fixes linking so that you cannot link to any non imported asset (which makes sense).

kitsunet commented 5 years ago

I changed this to a feature, as it would definitely be nice to have but at this point no such links should be created therefore it's not a bug but simply a missing feature that we cannot link to proxies.

kitsunet commented 5 years ago

To clarify, the necessary feature here would be to import the asset proxy just as it happens with images when you select one and then create the link on the imported (proxy) asset.