[MGNLETK-109] fckEditor: repository browser doesn't expand to linked node if working on multiple sites from one base domain Created: 24/Oct/13  Updated: 27/Nov/13  Resolved: 26/Nov/13

Status: Closed
Project: Extended Templating Kit (closed)
Component/s: None
Affects Version/s: None
Fix Version/s: 2.0.15

Type: Bug Priority: Major
Reporter: Christian Balaguer Assignee: Chunhua Liu
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Cloners
is cloned by MULTISITE-9 CLONE - fckEditor: repository browser... Closed
Relates
relates to MAGNOLIA-5250 fckEditor: make repository browser si... Closed
relation
is related to MGNLUI-2450 fckEditor: repository browser doesn't... Closed
Template:
Acceptance criteria:
Empty
Date of First Response:
Visible to:
Daniel Kummer, Raphael Joss, Stefan Baur
Epic Link: Support / 4.5.14
Sprint: 4.5.14

 Description   

The problem is, that the links don't get expanded if you request Magnolia via a domain which is not mapped to a sitedefinition.

To reproduce, follow these steps:

Case 1

  1. Visit http://demoauthor45.magnolia-cms.com/.magnolia/pages/adminCentral.html
  2. Verify that in the site definition of demo-project the domain mapping does not equal "demoauthor45.magnolia-cms.com"
  3. On the page /demo-project/about/subsection-articles/article edit a TextImage component and create an internal link to a page of the demo-project
  4. Click OK and save the dialog
  5. Reopen the dialog and edit the previously created link (notice that the prefixHandle got cut out)
  6. Click on "Browse Server"

Result: The tree is not expanded

Case 2

  1. Now verify that in the site definition of demo-project the domain mapping "demoauthor45.magnolia-cms.com" exists!
  2. Reopen the dialog, edit the link and click on "Browse Server"

Result: The tree is expanded, but only if you link to pages of the current website (crossdomain links don't expand the tree)

Can't we just always save the link as absolute URIs without cutting out the handlePrefix like it is done when you select a document from dms instead of a page from website tree?

Chris



 Comments   
Comment by Christian Balaguer [ 24/Oct/13 ]

This ticket is related to the following tickets:

Comment by Jan Haderka [ 31/Oct/13 ]

I'm not sure it is correct to switch off the use of uri mappings. Did you try what happens with binary or page resources when referenced in the editor itself? I believe they have incorrect links now when you create links to own site content.

Also for the test - please rather then "extending" existing test method, add new test method to make it easier to see the failures in the future. Right now if first test in the method fails, it would prevent other part from being executed thus hiding the test itself.

Comment by Christopher Zimmermann [ 01/Nov/13 ]

Here is some reference: http://documentation.magnolia-cms.com/display/DOCS/URI+mapping

Comment by Jan Haderka [ 01/Nov/13 ]

and some more to the story:

  • originally link editor transformer (LET) was not site aware. That was fixed (see linked ticket).
    • by site-aware I mean that it was not shortening URIs for links to the same site.
  • once LET was site aware, issue arose with repository browser which could no longer open shortened path
    • this was fixed in another ticket (see linked ticket)
  • this resulted in broken display of images (or content of other workspaces that were not working with site-mapping/url-shortening
    • again this was fixed in one of the linked tickets
  • and the current situation is that we have cross site links that are correctly generated, but repository browser is not able to resolve such links to full path and reopen the tree pointing to the correct content.

That being said I think that issue needs to be fixed in Repository browser if possible to minimise space for any negative side effects.

possible test cases:

  • content in SiteA has a link to another page in SiteA ==> generated URI needs to be shortened. When reopening repo browser, right page needs to be selected.
  • content in SiteA has a link to DMS/Data binary ==> generated URI is correct and not shortened. When reopening repo browser, right binary needs to be selected.
  • content in SiteA has a link to uploaded binary ==> generated URI is correct and shortened. No repo browser.
  • content in SiteA has a link to content in SiteB ==> generated URI is correct and NOT shortened. When reopening repo browser, right page needs to be selected. (this is actually the use case described in this ticket itself)
Comment by Cheng Hu [ 14/Nov/13 ]

Moved from MAGNOLIA-5417 to MGNLETK-107 because fix is in ETK module. Previous commits are logged to MAGNOLIA-5417.

Comment by Cheng Hu [ 14/Nov/13 ]

In Magnolia 5, the functionality no longer exists to jump to the tree location pointed to by a link in the editor when editing the link (perhaps this is an oversight?). For example, in a Text & Image component, create a link to a page in the tree using the Link to Magnolia Page button. Open the link again using the Link to Magnolia Page button. Regardless of whether domains are set correctly, the dialog does not jump to the page linked to in the tree.

As a result, this bug is no longer reproducible on Magnolia 5.

However, we have ported the changes from 2.0.15 to 2.x anyway because we believe it makes the logic of calculating an URI from a link more correct and also so that the logic for doing so is uniform between 4.5 and 5.

Comment by Cheng Hu [ 14/Nov/13 ]

We changed the logic to calculate an URI from a link into the website workspace in ETKURI2RepositoryManager.getURI(Link uuidLink).

Previously, the URI for all links are generated using the mappings defined in the site definition.

After the change, if the link is to the website workspace and the URI of the request context does not match any of the domains listed in the site definition, then mappings defined in the site definition are not applied. For links into any other workspace, and for links into the website workspace where the request context does match some domain listed in the site definition, mappings defined in the site definition are applied just as before.

Comment by Cheng Hu [ 14/Nov/13 ]

There is an issue caused by the fix above that we are aware of but think it is an issue with the existing code rather than our code.

How to reproduce the issue:

This assumes testing on the ee-bundle running in Eclipse with our code on localhost. The URI used to access demo-project is http://localhost:8080/magnolia-enterprise-webapp/.

In the site definition for demo project, in /demo-project/domains/demo-project/, add the following properties and values:

  • name = localhost
  • port = 9999
  • protocol = http
  • context = /magnolia-enterprise-webapp

Note that the port is incorrect. Open the about page at http://localhost:8080/magnolia-enterprise-webapp/about.html, see that it does not display correctly.

Now change the domain definition by changing the name property while keeping other properties the same:

  • name = xyz
  • port = 9999
  • protocol = http
  • context = /magnolia-enterprise-webapp

Open the about page at http://localhost:8080/magnolia-enterprise-webapp/about.html, see that it does display correctly.

Reasoning for why this is a problem in the existing code:

In both cases above, our code correctly decides that the domain definition does not match the request context, and returns the same URI from a link. That is, in both cases, our code returns the same thing.

This means that changing the name property causes some change in the existing code that causes the different results. However, simply changing the name property should have no effect, because the port property is always wrong and the domain definition should never affect anything in either case.

Theory as to what is wrong

One theory is that somewhere in the existing code, there is also a decision being made as to whether a domain definition applies to the current request context. However, that is only checking whether the name matches, but does not check any of the other properties such as port or context or protocol.

Possible fixes

One fix is that we can make our code emulate the incorrect behavior and decide that a domain definition matches when just the name property matches. We have tried this and find that it solves the above problem. However, it may not address the real problem.

The other fix is to find the place in the current code that decides whether a domain definition applies but only checks the name property, assuming the theory is correct, and change it to also check the other three properties.

Comment by Jan Haderka [ 20/Nov/13 ]
  • making changes in ETKURI2RepositoryManager is a change that will impact other parts of Magnolia. I think it is too far reaching for fixing this issue. Fix needs to happen much closer to the problem.
  • the domain checking in the code on branch can be simplified, rather then taking again domain, port and all other info, you should rely on site = ((ExtendedAggregationState) MgnlContext.getAggregationState()).getSite(); to give you already resolved site using whatever rules are set in SiteMappingFilter so that the resolution is consistent. And last but not least. Imho the problem in editor is that the "current" request is to the editor (hence always pointing to the default site), while the mapping needs to be resolved between page that is being edited and target of the links. It's the fact that those 2 belong to different site that causes problems. It would seem to me that we check based on edited page (source of the link) which site we are on, rather then based on the target.

All things said I would think fix should be located in custom RepoBrowserPage or in EditorLinkTransformer.

Comment by Cheng Hu [ 22/Nov/13 ]

Moved from MGNLETK-107 to MAGNOLIA-5510 because fix is in core module. Previous commits are logged to MAGNOLIA-5417 and MGNLETK-107.

Comment by Jan Haderka [ 25/Nov/13 ]

After sleeping on it for few days, I realised there is still at least one extra case that this fix doesn't handle and that's the situation when you configure micro site (i.e. sub site of given site that has same domain name, but uses different path mapping in <site name>/mappings/website/handlePrefix. While you could probably extend the code to check not only domain names, but also mapping, it just shows how brittle is the fix and how it would break every time we adjust mechanism for site resolution.
That being said I would suggest following:

  • leave EditorLinkTransformer that is in the main project untouched.
  • create new STKEditorLinkTransformer that would exist in STK or ETK so it would be able to use site manager for resolving correct site
  • configure this new transformer to be used as editor transformer in ETKLinkTransformerManager/MultiSiteLinkTRansformerManager (It's configured under config:/server/rendering/linkManagement)

And fix needs to be applied on both 4.5.x and 5.2.x branches.

Sorry for not realising this is the proper course of action earlier.

Comment by Jan Haderka [ 26/Nov/13 ]

Perfect and again sorry, for having to redo it multiple times.

Just one tiny detail - since fix is now in ETK/Multisite, you should move & clone the issue to their respective projects instead of keeping it in main project.

Comment by Cheng Hu [ 27/Nov/13 ]

Moved from MAGNOLIA-5510 to MGNLETK-109 because fix is in ETK module. Previous commits are logged to MAGNOLIA-5417, MGNLETK-107, and MAGNOLIA-5510.

Comment by Christopher Zimmermann [ 27/Nov/13 ]

QA passed on bundle.

Generated at Mon Feb 12 01:48:33 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.