Skip to content

Commit

Permalink
Fix HasMenuCurrent and IsDescendant/IsAncestor when comparing to itself
Browse files Browse the repository at this point in the history
There may be sites in the wild that depends on the faulty behaviour of IsDescendant/IsAncestor when comparing to itself, but

* The documentation and common sense says that a thing cannot be descendant or ancestor to itself.
* The bug introduced in `HasMenuCurrent` comes directly from that confusion.

Fixes #9846
  • Loading branch information
bep committed May 28, 2022
1 parent f343b8e commit 3b478f5
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 12 deletions.
8 changes: 4 additions & 4 deletions hugolib/content_map_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -383,8 +383,8 @@ Blog Section: {{ template "print-page" $blog }}
Blog Sub Section: {{ template "print-page" $blogSub }}
Page: {{ template "print-page" $page }}
Bundle: {{ template "print-page" $bundle }}
IsDescendant: true: {{ $page.IsDescendant $blog }} true: {{ $blogSub.IsDescendant $blog }} true: {{ $bundle.IsDescendant $blog }} true: {{ $page4.IsDescendant $blog }} true: {{ $blog.IsDescendant $home }} true: {{ $blog.IsDescendant $blog }} false: {{ $home.IsDescendant $blog }}
IsAncestor: true: {{ $blog.IsAncestor $page }} true: {{ $home.IsAncestor $blog }} true: {{ $blog.IsAncestor $blogSub }} true: {{ $blog.IsAncestor $bundle }} true: {{ $blog.IsAncestor $page4 }} true: {{ $home.IsAncestor $page }} true: {{ $blog.IsAncestor $blog }} false: {{ $page.IsAncestor $blog }} false: {{ $blog.IsAncestor $home }} false: {{ $blogSub.IsAncestor $blog }}
IsDescendant: true: {{ $page.IsDescendant $blog }} true: {{ $blogSub.IsDescendant $blog }} true: {{ $bundle.IsDescendant $blog }} true: {{ $page4.IsDescendant $blog }} true: {{ $blog.IsDescendant $home }} false: {{ $blog.IsDescendant $blog }} false: {{ $home.IsDescendant $blog }}
IsAncestor: true: {{ $blog.IsAncestor $page }} true: {{ $home.IsAncestor $blog }} true: {{ $blog.IsAncestor $blogSub }} true: {{ $blog.IsAncestor $bundle }} true: {{ $blog.IsAncestor $page4 }} true: {{ $home.IsAncestor $page }} false: {{ $blog.IsAncestor $blog }} false: {{ $page.IsAncestor $blog }} false: {{ $blog.IsAncestor $home }} false: {{ $blogSub.IsAncestor $blog }}
IsDescendant overlap1: false: {{ $overlap1.IsDescendant $overlap2 }}
IsDescendant overlap2: false: {{ $overlap2.IsDescendant $overlap1 }}
IsAncestor overlap1: false: {{ $overlap1.IsAncestor $overlap2 }}
Expand Down Expand Up @@ -426,8 +426,8 @@ Draft5: {{ if (.Site.GetPage "blog/draftsection/sub/page") }}FOUND{{ end }}|
Blog Sub Section: Page 3|/blog/subsection/|2019-06-03|Current Section: blog/subsection|Resources: application: /blog/subsection/subdata.json|
Page: Page 1|/blog/page1/|2019-06-01|Current Section: blog|Resources:
Bundle: Page 12|/blog/bundle/|0001-01-01|Current Section: blog|Resources: application: /blog/bundle/data.json|page: |
IsDescendant: true: true true: true true: true true: true true: true true: true false: false
IsAncestor: true: true true: true true: true true: true true: true true: true true: true false: false false: false false: false
IsDescendant: true: true true: true true: true true: true true: true false: false false: false
IsAncestor: true: true true: true true: true true: true true: true true: true false: false false: false false: false false: false
IsDescendant overlap1: false: false
IsDescendant overlap2: false: false
IsAncestor overlap1: false: false
Expand Down
64 changes: 64 additions & 0 deletions hugolib/menu_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -524,3 +524,67 @@ Blog|HasMenuCurrent: false|Page: Page(/blog/_index.md)
Blog|IsMenuCurrent: false|Page: Page(/blog/_index.md)
`)
}

// Issue 9846
func TestMenuHasMenuCurrentSection(t *testing.T) {
t.Parallel()

files := `
-- config.toml --
disableKinds = ['RSS','sitemap','taxonomy','term']
[[menu.main]]
name = 'Home'
pageRef = '/'
weight = 1
[[menu.main]]
name = 'Tests'
pageRef = '/tests'
weight = 2
[[menu.main]]
name = 'Test 1'
pageRef = '/tests/test-1'
parent = 'Tests'
weight = 1
-- content/tests/test-1.md --
---
title: "Test 1"
---
-- layouts/_default/list.html --
{{ range site.Menus.main }}
{{ .Name }}|{{ .URL }}|IsMenuCurrent = {{ $.IsMenuCurrent "main" . }}|HasMenuCurrent = {{ $.HasMenuCurrent "main" . }}|
{{ range .Children }}
{{ .Name }}|{{ .URL }}|IsMenuCurrent = {{ $.IsMenuCurrent "main" . }}|HasMenuCurrent = {{ $.HasMenuCurrent "main" . }}|
{{ end }}
{{ end }}
{{/* Some tests for issue 9925 */}}
{{ $page := .Site.GetPage "tests/test-1" }}
{{ $section := site.GetPage "tests" }}
Home IsAncestor Self: {{ site.Home.IsAncestor site.Home }}
Home IsDescendant Self: {{ site.Home.IsDescendant site.Home }}
Section IsAncestor Self: {{ $section.IsAncestor $section }}
Section IsDescendant Self: {{ $section.IsDescendant $section}}
Page IsAncestor Self: {{ $page.IsAncestor $page }}
Page IsDescendant Self: {{ $page.IsDescendant $page}}
`

b := NewIntegrationTestBuilder(
IntegrationTestConfig{
T: t,
TxtarString: files,
},
).Build()

b.AssertFileContent("public/tests/index.html", `
Tests|/tests/|IsMenuCurrent = true|HasMenuCurrent = false
Home IsAncestor Self: false
Home IsDescendant Self: false
Section IsAncestor Self: false
Section IsDescendant Self: false
Page IsAncestor Self: false
Page IsDescendant Self: false
`)
}
14 changes: 6 additions & 8 deletions hugolib/page__tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ func (pt pageTree) IsAncestor(other any) (bool, error) {
}

ref1, ref2 := pt.p.getTreeRef(), tp.getTreeRef()
if ref1 != nil && ref2 != nil && ref1.key == ref2.key {
return false, nil
}

if ref1 != nil && ref1.key == "/" {
return true, nil
Expand All @@ -50,10 +53,6 @@ func (pt pageTree) IsAncestor(other any) (bool, error) {
return ref1.n.p.IsHome(), nil
}

if ref1.key == ref2.key {
return true, nil
}

if strings.HasPrefix(ref2.key, ref1.key) {
return true, nil
}
Expand Down Expand Up @@ -82,6 +81,9 @@ func (pt pageTree) IsDescendant(other any) (bool, error) {
}

ref1, ref2 := pt.p.getTreeRef(), tp.getTreeRef()
if ref1 != nil && ref2 != nil && ref1.key == ref2.key {
return false, nil
}

if ref2 != nil && ref2.key == "/" {
return true, nil
Expand All @@ -96,10 +98,6 @@ func (pt pageTree) IsDescendant(other any) (bool, error) {
return ref2.n.p.IsHome(), nil
}

if ref1.key == ref2.key {
return true, nil
}

if strings.HasPrefix(ref1.key, ref2.key) {
return true, nil
}
Expand Down

0 comments on commit 3b478f5

Please sign in to comment.