Fix IsMenuCurrent for SectionPagesMenu

Pretty sure it has worked at some point, but that PR probably has been rebased to pieces.

This refactors the fix by @dannys42 into a method, as this URL fix is applied several places.

Fixes #1114

hugolib/menu_test.go

@@ -319,6 +319,52 @@ func doTestMenuWithUnicodeURLs(t *testing.T, canonifyURLs, uglyURLs bool) {
 	assert.Equal(t, expected, unicodeRussian.URL, "uglyURLs[%t]", uglyURLs)
 }
 
+// Issue #1114
+func TestSectionPagesMenu(t *testing.T) {
+	viper.Set("SectionPagesMenu", "spm")
+	defer viper.Set("SectionPagesMenu", "")
+
+	for _, canonify := range []bool{true, false} {
+		doTestSectionPagesMenu(canonify, t)
+	}
+}
+
+func doTestSectionPagesMenu(canonifyUrls bool, t *testing.T) {
+	viper.Set("CanonifyURLs", canonifyUrls)
+	ts := setupMenuTests(t, MENU_PAGE_SECTIONS_SOURCES)
+	defer resetMenuTestState(ts)
+
+	assert.Equal(t, 2, len(ts.site.Sections))
+
+	firstSectionPages := ts.site.Sections["first"]
+	assert.Equal(t, 2, len(firstSectionPages))
+	secondSectionPages := ts.site.Sections["second-section"]
+	assert.Equal(t, 1, len(secondSectionPages))
+
+	nodeFirst := ts.site.newSectionListNode("first", firstSectionPages)
+	nodeSecond := ts.site.newSectionListNode("second-section", secondSectionPages)
+
+	firstSectionMenuEntry := ts.findTestMenuEntryByID("spm", "first")
+	secondSectionMenuEntry := ts.findTestMenuEntryByID("spm", "second-section")
+
+	assert.NotNil(t, firstSectionMenuEntry)
+	assert.NotNil(t, secondSectionMenuEntry)
+	assert.NotNil(t, nodeFirst)
+	assert.NotNil(t, nodeSecond)
+
+	for _, p := range firstSectionPages {
+		assert.True(t, p.Page.HasMenuCurrent("spm", firstSectionMenuEntry))
+		assert.False(t, p.Page.HasMenuCurrent("spm", secondSectionMenuEntry))
+		assert.True(t, nodeFirst.IsMenuCurrent("spm", firstSectionMenuEntry))
+		assert.False(t, nodeFirst.IsMenuCurrent("spm", secondSectionMenuEntry))
+	}
+
+	for _, p := range secondSectionPages {
+		assert.False(t, p.Page.HasMenuCurrent("spm", firstSectionMenuEntry))
+		assert.True(t, p.Page.HasMenuCurrent("spm", secondSectionMenuEntry))
+	}
+}
+
 func TestTaxonomyNodeMenu(t *testing.T) {
 	viper.Set("CanonifyURLs", true)
 	ts := setupMenuTests(t, MENU_PAGE_SOURCES)

hugolib/node.go

@@ -18,7 +18,6 @@ import (
 	"html/template"
 	"sync"
 	"time"
-    "strings"
 )
 
 type Node struct {
@@ -57,19 +56,8 @@ func (n *Node) HasMenuCurrent(menuID string, inme *MenuEntry) bool {
 }
 
 func (n *Node) IsMenuCurrent(menuID string, inme *MenuEntry) bool {
-    s := n.Site
 
-	me := MenuEntry{Name: n.Title, URL: n.URL}
+	me := MenuEntry{Name: n.Title, URL: n.Site.createNodeMenuEntryURL(n.URL)}
-
-	if strings.HasPrefix(me.URL, "/") {
-		// make it match the nodes
-		menuEntryURL := me.URL
-		menuEntryURL = helpers.URLizeAndPrep(menuEntryURL)
-		if !s.canonifyURLs {
-		    menuEntryURL = helpers.AddContextRoot(string(s.BaseURL), menuEntryURL)
-		}
-		me.URL = menuEntryURL
-	}
 
 	if !me.IsSameResource(inme) {
 		return false

hugolib/site.go

@@ -724,16 +724,7 @@ func (s *Site) getMenusFromConfig() Menus {
 					}
 
 					menuEntry.MarshallMap(ime)
-
+					menuEntry.URL = s.Info.createNodeMenuEntryURL(menuEntry.URL)
-					if strings.HasPrefix(menuEntry.URL, "/") {
-						// make it match the nodes
-						menuEntryURL := menuEntry.URL
-						menuEntryURL = helpers.URLizeAndPrep(menuEntryURL)
-						if !s.Info.canonifyURLs {
-							menuEntryURL = helpers.AddContextRoot(string(s.Info.BaseURL), menuEntryURL)
-						}
-						menuEntry.URL = menuEntryURL
-					}
 
 					if ret[name] == nil {
 						ret[name] = &Menu{}
@@ -747,6 +738,20 @@ func (s *Site) getMenusFromConfig() Menus {
 	return ret
 }
 
+func (s *SiteInfo) createNodeMenuEntryURL(in string) string {
+
+	if !strings.HasPrefix(in, "/") {
+		return in
+	}
+	// make it match the nodes
+	menuEntryURL := in
+	menuEntryURL = helpers.URLizeAndPrep(menuEntryURL)
+	if !s.canonifyURLs {
+		menuEntryURL = helpers.AddContextRoot(string(s.BaseURL), menuEntryURL)
+	}
+	return menuEntryURL
+}
+
 func (s *Site) assembleMenus() {
 
 	type twoD struct {
@@ -770,7 +775,7 @@ func (s *Site) assembleMenus() {
 		if sectionPagesMenu != "" {
 			if _, ok := sectionPagesMenus[p.Section()]; !ok {
 				if p.Section() != "" {
-					me := MenuEntry{Identifier: p.Section(), Name: helpers.MakeTitle(p.Section()), URL: s.permalinkStr(p.Section())}
+					me := MenuEntry{Identifier: p.Section(), Name: helpers.MakeTitle(p.Section()), URL: s.Info.createNodeMenuEntryURL("/" + p.Section())}
 					if _, ok := flat[twoD{sectionPagesMenu, me.KeyName()}]; ok {
 						// menu with same id defined in config, let that one win
 						continue