From 1b3782f2b0ad8533101d2a498944acca345ce6c0 Mon Sep 17 00:00:00 2001 From: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com> Date: Tue, 26 Nov 2024 09:08:22 +0100 Subject: [PATCH 01/10] fix: should check specifically for if pathname is undefined or empty --- .../packages/core/utils/path/has-own-opener.function.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/has-own-opener.function.ts b/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/has-own-opener.function.ts index 3fda364240..5d4ee2ce53 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/has-own-opener.function.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/has-own-opener.function.ts @@ -22,11 +22,12 @@ export function hasOwnOpener(pathname?: string, windowLike: Window = globalThis. return false; } - if (pathname && openerLocation.pathname.startsWith(pathname)) { - return true; + // If there is a pathname, check if the opener has the same pathname + if (typeof pathname !== 'undefined' && !openerLocation.pathname.startsWith(pathname)) { + return false; } - return false; + return true; } catch { // If there is a security error, it means that the opener is from a different origin, so we let it fall through return false; From 72596bd853544847ff038944d0a09b75ee1416e8 Mon Sep 17 00:00:00 2001 From: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com> Date: Tue, 26 Nov 2024 09:08:22 +0100 Subject: [PATCH 02/10] fix: should check specifically for if pathname is undefined or empty --- .../packages/core/utils/path/has-own-opener.function.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/has-own-opener.function.ts b/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/has-own-opener.function.ts index 3fda364240..5d4ee2ce53 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/has-own-opener.function.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/has-own-opener.function.ts @@ -22,11 +22,12 @@ export function hasOwnOpener(pathname?: string, windowLike: Window = globalThis. return false; } - if (pathname && openerLocation.pathname.startsWith(pathname)) { - return true; + // If there is a pathname, check if the opener has the same pathname + if (typeof pathname !== 'undefined' && !openerLocation.pathname.startsWith(pathname)) { + return false; } - return false; + return true; } catch { // If there is a security error, it means that the opener is from a different origin, so we let it fall through return false; From 2846325a8a18e7572348d9fd083ced9a2de24720 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Tue, 26 Nov 2024 09:55:51 +0100 Subject: [PATCH 03/10] Fixes routing issues (#17572) * Child of second root should also hide root path and backoffice needs to show all domains for each language. * Fixes routing issues based on findings https://github.com/umbraco/Umbraco-CMS/pull/17572#issuecomment-2486219574 * Revert "Fixes routing issues based on findings https://github.com/umbraco/Umbraco-CMS/pull/17572#issuecomment-2486219574" This reverts commit ba7fb5cc904fbe602450ac109e1821282b7f69d6. * Fix urls of descendants of non-first roots do not show the root urlsegment when HideTopLevel is true --- .../Services/DocumentUrlService.cs | 191 +++++++++++++----- ...ument-workspace-view-info-links.element.ts | 29 +-- .../Services/DocumentUrlServiceTest.cs | 39 ++++ ...cumentUrlServiceTest_hidetoplevel_false.cs | 29 +++ 4 files changed, 222 insertions(+), 66 deletions(-) diff --git a/src/Umbraco.Core/Services/DocumentUrlService.cs b/src/Umbraco.Core/Services/DocumentUrlService.cs index 95e93c58eb..58af9805f1 100644 --- a/src/Umbraco.Core/Services/DocumentUrlService.cs +++ b/src/Umbraco.Core/Services/DocumentUrlService.cs @@ -393,7 +393,19 @@ public class DocumentUrlService : IDocumentUrlService return GetTopMostRootKey(isDraft, culture); } - // Otherwise we have to find the root items (or child of the first root when hideTopLevelNodeFromPath is true) and follow the url segments in them to get to correct document key + // Special case for all top level nodes except the first (that will have /) + if (runnerKey is null && urlSegments.Length == 1 && hideTopLevelNodeFromPath is true) + { + IEnumerable rootKeys = GetKeysInRoot(false, isDraft, culture); + Guid? rootKeyWithUrlSegment = GetChildWithUrlSegment(rootKeys, urlSegments.First(), culture, isDraft); + + if (rootKeyWithUrlSegment is not null) + { + return rootKeyWithUrlSegment; + } + } + + // Otherwise we have to find the root items (or child of the roots when hideTopLevelNodeFromPath is true) and follow the url segments in them to get to correct document key for (var index = 0; index < urlSegments.Length; index++) { var urlSegment = urlSegments[index]; @@ -447,7 +459,7 @@ public class DocumentUrlService : IDocumentUrlService var cultureOrDefault = string.IsNullOrWhiteSpace(culture) is false ? culture : _languageService.GetDefaultIsoCodeAsync().GetAwaiter().GetResult(); Guid[] ancestorsOrSelfKeysArray = ancestorsOrSelfKeys as Guid[] ?? ancestorsOrSelfKeys.ToArray(); - IDictionary ancestorOrSelfKeyToDomains = ancestorsOrSelfKeysArray.ToDictionary(x => x, ancestorKey => + ILookup ancestorOrSelfKeyToDomains = ancestorsOrSelfKeysArray.ToLookup(x => x, ancestorKey => { Attempt idAttempt = _idKeyMap.GetIdForKey(ancestorKey, UmbracoObjectTypes.Document); @@ -466,13 +478,11 @@ public class DocumentUrlService : IDocumentUrlService foreach (Guid ancestorOrSelfKey in ancestorsOrSelfKeysArray) { - if (ancestorOrSelfKeyToDomains.TryGetValue(ancestorOrSelfKey, out Domain? domain)) + var domains = ancestorOrSelfKeyToDomains[ancestorOrSelfKey].WhereNotNull(); + if (domains.Any()) { - if (domain is not null) - { - foundDomain = domain; - break; - } + foundDomain = domains.First();// What todo here that is better? + break; } if (_cache.TryGetValue(CreateCacheKey(ancestorOrSelfKey, cultureOrDefault, isDraft), out PublishedDocumentUrlSegment? publishedDocumentUrlSegment)) @@ -528,7 +538,7 @@ public class DocumentUrlService : IDocumentUrlService var cultures = languages.ToDictionary(x=>x.IsoCode); Guid[] ancestorsOrSelfKeysArray = ancestorsOrSelfKeys as Guid[] ?? ancestorsOrSelfKeys.ToArray(); - Dictionary>> ancestorOrSelfKeyToDomains = ancestorsOrSelfKeysArray.ToDictionary(x => x, async ancestorKey => + Dictionary>> ancestorOrSelfKeyToDomains = ancestorsOrSelfKeysArray.ToDictionary(x => x, async ancestorKey => { Attempt idAttempt = _idKeyMap.GetIdForKey(ancestorKey, UmbracoObjectTypes.Document); @@ -537,35 +547,43 @@ public class DocumentUrlService : IDocumentUrlService return null; } IEnumerable domains = _domainCacheService.GetAssigned(idAttempt.Result, false); - return domains.ToDictionary(x => x.Culture!); + return domains.ToLookup(x => x.Culture!); })!; foreach ((string culture, ILanguage language) in cultures) { - var urlSegments = new List(); - Domain? foundDomain = null; + var urlSegments = new List(); + List foundDomains = new List(); - var hasUrlInCulture = true; - foreach (Guid ancestorOrSelfKey in ancestorsOrSelfKeysArray) - { - if (ancestorOrSelfKeyToDomains.TryGetValue(ancestorOrSelfKey, out Task>? domainDictionaryTask)) + var hasUrlInCulture = true; + foreach (Guid ancestorOrSelfKey in ancestorsOrSelfKeysArray) + { + var domainLookup = await ancestorOrSelfKeyToDomains[ancestorOrSelfKey]; + if (domainLookup.Any()) { - Dictionary domainDictionary = await domainDictionaryTask; - if (domainDictionary.TryGetValue(culture, out Domain? domain)) + var domains = domainLookup[culture]; + foreach (Domain domain in domains) { - Attempt domainKeyAttempt = _idKeyMap.GetKeyForId(domain.ContentId, UmbracoObjectTypes.Document); + Attempt domainKeyAttempt = + _idKeyMap.GetKeyForId(domain.ContentId, UmbracoObjectTypes.Document); if (domainKeyAttempt.Success) { if (_publishStatusQueryService.IsDocumentPublished(domainKeyAttempt.Result, culture)) { - foundDomain = domain; - break; + foundDomains.Add(domain); } } } + + if (foundDomains.Any()) + { + break; + } } - if (_cache.TryGetValue(CreateCacheKey(ancestorOrSelfKey, culture, false), out PublishedDocumentUrlSegment? publishedDocumentUrlSegment)) + if (_cache.TryGetValue( + CreateCacheKey(ancestorOrSelfKey, culture, false), + out PublishedDocumentUrlSegment? publishedDocumentUrlSegment)) { urlSegments.Add(publishedDocumentUrlSegment.UrlSegment); } @@ -573,34 +591,75 @@ public class DocumentUrlService : IDocumentUrlService { hasUrlInCulture = false; } - } + } - //If we did not find a domain and this is not the default language, then the content is not routable - if (foundDomain is null && language.IsDefault is false) - { - continue; - } + //If we did not find a domain and this is not the default language, then the content is not routable + if (foundDomains.Any() is false && language.IsDefault is false) + { + continue; + } - var isRootFirstItem = GetTopMostRootKey(false, culture) == ancestorsOrSelfKeysArray.Last(); - var leftToRight = _globalSettings.ForceCombineUrlPathLeftToRight - || CultureInfo.GetCultureInfo(culture).TextInfo.IsRightToLeft is false; - if (leftToRight) - { - urlSegments.Reverse(); - } + var isRootFirstItem = GetTopMostRootKey(false, culture) == ancestorsOrSelfKeysArray.Last(); - result.Add(new UrlInfo( - text: GetFullUrl(isRootFirstItem, urlSegments, foundDomain, leftToRight), - isUrl: hasUrlInCulture, - culture: culture - )); + var leftToRight = _globalSettings.ForceCombineUrlPathLeftToRight + || CultureInfo.GetCultureInfo(culture).TextInfo.IsRightToLeft is false; + if (leftToRight) + { + urlSegments.Reverse(); + } + // If no domain was found, we need to add a null domain to the list to make sure we check for no domains. + if (foundDomains.Any() is false) + { + foundDomains.Add(null); + } + + foreach (Domain? foundDomain in foundDomains) + { + var foundUrl = GetFullUrl(isRootFirstItem, urlSegments, foundDomain, leftToRight); + + if (foundDomain is not null) + { + // if we found a domain, it should be safe to show url + result.Add(new UrlInfo( + text: foundUrl, + isUrl: hasUrlInCulture, + culture: culture + )); + } + else + { + // otherwise we need to ensure that no other page has the same url + // e.g. a site with two roots that both have a child with the same name + Guid? documentKeyByRoute = GetDocumentKeyByRoute(foundUrl, culture, foundDomain?.ContentId, false); + if (contentKey.Equals(documentKeyByRoute)) + { + result.Add(new UrlInfo( + text: foundUrl, + isUrl: hasUrlInCulture, + culture: culture + )); + } + else + { + result.Add(new UrlInfo( + text: "Conflict: Other page has the same url", + isUrl: false, + culture: culture + )); + } + } + + + + } } return result; } + private string GetFullUrl(bool isRootFirstItem, List segments, Domain? foundDomain, bool leftToRight) { var urlSegments = new List(segments); @@ -610,7 +669,7 @@ public class DocumentUrlService : IDocumentUrlService return foundDomain.Name.EnsureEndsWith("/") + string.Join('/', urlSegments); } - var hideTopLevel = _globalSettings.HideTopLevelNodeFromPath && isRootFirstItem; + var hideTopLevel = HideTopLevel(_globalSettings.HideTopLevelNodeFromPath, isRootFirstItem, urlSegments); if (leftToRight) { return '/' + string.Join('/', urlSegments.Skip(hideTopLevel ? 1 : 0)); @@ -624,6 +683,21 @@ public class DocumentUrlService : IDocumentUrlService return '/' + string.Join('/', urlSegments); } + private bool HideTopLevel(bool hideTopLevelNodeFromPath, bool isRootFirstItem, List urlSegments) + { + if (hideTopLevelNodeFromPath is false) + { + return false; + } + + if(isRootFirstItem is false && urlSegments.Count == 1) + { + return false; + } + + return true; + } + public async Task CreateOrUpdateUrlSegmentsWithDescendantsAsync(Guid key) { var id = _idKeyMap.GetIdForKey(key, UmbracoObjectTypes.Document).Result; @@ -646,7 +720,7 @@ public class DocumentUrlService : IDocumentUrlService } } - private IEnumerable GetKeysInRoot(bool addFirstLevelChildren, bool isDraft, string culture) + private IEnumerable GetKeysInRoot(bool considerFirstLevelAsRoot, bool isDraft, string culture) { if (_documentNavigationQueryService.TryGetRootKeys(out IEnumerable rootKeysEnumerable) is false) { @@ -655,13 +729,10 @@ public class DocumentUrlService : IDocumentUrlService IEnumerable rootKeys = rootKeysEnumerable as Guid[] ?? rootKeysEnumerable.ToArray(); - foreach (Guid rootKey in rootKeys) + if (considerFirstLevelAsRoot) { - yield return rootKey; - } + yield return rootKeys.First(); - if (addFirstLevelChildren) - { foreach (Guid rootKey in rootKeys) { if (isDraft is false && IsContentPublished(rootKey, culture) is false) @@ -677,6 +748,13 @@ public class DocumentUrlService : IDocumentUrlService } } } + else + { + foreach (Guid rootKey in rootKeys) + { + yield return rootKey; + } + } } @@ -710,11 +788,7 @@ public class DocumentUrlService : IDocumentUrlService return Enumerable.Empty(); } - /// - /// Gets the top most root key. - /// - /// The top most root key. - private Guid? GetTopMostRootKey(bool isDraft, string culture) + private IEnumerable GetRootKeys(bool isDraft, string culture) { if (_documentNavigationQueryService.TryGetRootKeys(out IEnumerable rootKeys)) { @@ -722,11 +796,20 @@ public class DocumentUrlService : IDocumentUrlService { if (isDraft || IsContentPublished(rootKey, culture)) { - return rootKey; + yield return rootKey; } } } - return null; + } + + + /// + /// Gets the top most root key. + /// + /// The top most root key. + private Guid? GetTopMostRootKey(bool isDraft, string culture) + { + return GetRootKeys(isDraft, culture).Cast().FirstOrDefault(); } [MethodImpl(MethodImplOptions.AggressiveInlining)] diff --git a/src/Umbraco.Web.UI.Client/src/packages/documents/documents/workspace/views/info/document-workspace-view-info-links.element.ts b/src/Umbraco.Web.UI.Client/src/packages/documents/documents/workspace/views/info/document-workspace-view-info-links.element.ts index 7165a5a786..18e6b48db9 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/documents/documents/workspace/views/info/document-workspace-view-info-links.element.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/documents/documents/workspace/views/info/document-workspace-view-info-links.element.ts @@ -22,7 +22,7 @@ export class UmbDocumentWorkspaceViewInfoLinksElement extends UmbLitElement { private _variantOptions?: Array; @state() - private _lookup: Record = {}; + private _lookup: Record = {}; constructor() { super(); @@ -56,7 +56,10 @@ export class UmbDocumentWorkspaceViewInfoLinksElement extends UmbLitElement { if (data?.length) { data[0].urls.forEach((item) => { if (item.culture && item.url) { - this._lookup[item.culture] = item.url; + if(this._lookup[item.culture] == null){ + this._lookup[item.culture] = []; + } + this._lookup[item.culture].push(item.url); } }); this.requestUpdate('_lookup'); @@ -108,18 +111,20 @@ export class UmbDocumentWorkspaceViewInfoLinksElement extends UmbLitElement { #renderUrl(variantOption: UmbDocumentVariantOptionModel) { const varies = !!variantOption.culture; const culture = varies ? variantOption.culture! : variantOption.language.unique; - const url = this._lookup[culture]; + const urls = this._lookup[culture]; return when( - url, + urls && urls.length >= 1, () => html` - - - ${varies ? culture : nothing} - ${url} - - - - `, + ${urls.map((url) => + html` + + + ${varies ? culture : nothing} + ${url} + + + ` + )}`, () => html`