Fix RelPermalink() and Urls in menus vs canonifyUrls

canonifyUrls=true, RelPermalink and baseUrl with sub-path did not work.

This fixes that by adding a check for canonifyUrl=trues=true in RelPermalink().

So given

- baseUrl "http://somehost.com/sub/"
- the path "some-path/file.html"

For canonifyUrls=false RelPermalink() returns "/sub/some-path/file.html"
For canonifyUrls=true RelPermalink() returns "/some-path/file.html"

In the last case, the Url will be made absolute and clickable in a later step.

This commit also makes the menu urls defined in site config releative. To make them work with canonifying of urls, the context root is prepended if canonifying is turned off.

Fixes #519
Fixes #711

docs/content/extras/menus.md

@@ -96,10 +96,12 @@ Here’s an example `config.toml`:
         pre = "<i class='fa fa-heart'></i>"
         weight = -110
         identifier = "about"
+        url = "/about/"
     [[menu.main]]
         name = "getting started"
         pre = "<i class='fa fa-road'></i>"
         weight = -100
+        url = "/getting-started/"
 
 And the equivalent example `config.yaml`:
 
@@ -110,11 +112,16 @@ And the equivalent example `config.yaml`:
             Pre: "<i class='fa fa-heart'></i>"
             Weight: -110
             Identifier: "about"
+            Url: "/about/"
           - Name: "getting started"
             Pre: "<i class='fa fa-road'></i>"
             Weight: -100
+            Url: "/getting-started/"
     ---            
 
+
+**NOTE:** The urls must be relative to the context root. If the `BaseUrl` is `http://example.com/mysite/`, then the urls in the menu must not include the context root `mysite`. 
+  
 ## Nesting
 
 All nesting of content is done via the `parent` field.

helpers/url.go

@@ -78,6 +78,25 @@ func MakePermalink(host, plink string) *url.URL {
 	return base
 }
 
+// AddContextRoot adds the context root to an URL if it's not already set.
+// For relative URL entries on sites with a base url with a context root set (i.e. http://example.com/mysite),
+// relative URLs must not include the context root if canonifyUrls is enabled. But if it's disabled, it must be set.
+func AddContextRoot(baseUrl, relativePath string) string {
+
+	url, err := url.Parse(baseUrl)
+	if err != nil {
+		panic(err)
+	}
+
+	newPath := path.Join(url.Path, relativePath)
+
+	// path strips traling slash
+	if strings.HasSuffix(relativePath, "/") {
+		newPath += "/"
+	}
+	return newPath
+}
+
 func UrlPrep(ugly bool, in string) string {
 	if ugly {
 		x := Uglify(SanitizeUrl(in))

helpers/url_test.go

@@ -68,6 +68,29 @@ func TestUrlPrep(t *testing.T) {
 
 }
 
+func TestAddContextRoot(t *testing.T) {
+	tests := []struct {
+		baseUrl  string
+		url      string
+		expected string
+	}{
+		{"http://example.com/sub/", "/foo", "/sub/foo"},
+		{"http://example.com/sub/", "/foo/index.html", "/sub/foo/index.html"},
+		{"http://example.com/sub1/sub2", "/foo", "/sub1/sub2/foo"},
+		{"http://example.com", "/foo", "/foo"},
+		// cannot guess that the context root is already added int the example below
+		{"http://example.com/sub/", "/sub/foo", "/sub/sub/foo"},
+		{"http://example.com/тря", "/трям/", "/тря/трям/"},
+	}
+
+	for _, test := range tests {
+		output := AddContextRoot(test.baseUrl, test.url)
+		if output != test.expected {
+			t.Errorf("Expected %#v, got %#v\n", test.expected, output)
+		}
+	}
+}
+
 func TestPretty(t *testing.T) {
 	assert.Equal(t, PrettifyUrlPath("/section/name.html"), "/section/name/index.html")
 	assert.Equal(t, PrettifyUrlPath("/section/sub/name.html"), "/section/sub/name/index.html")

hugolib/menu_test.go

@@ -265,18 +265,27 @@ func TestPageMenu(t *testing.T) {
 // issue #719
 func TestMenuWithUnicodeUrls(t *testing.T) {
 	for _, uglyUrls := range []bool{true, false} {
-		doTestMenuWithUnicodeUrls(t, uglyUrls)
+		for _, canonifyUrls := range []bool{true, false} {
+			doTestMenuWithUnicodeUrls(t, canonifyUrls, uglyUrls)
+		}
 	}
 }
 
-func doTestMenuWithUnicodeUrls(t *testing.T, uglyUrls bool) {
+func doTestMenuWithUnicodeUrls(t *testing.T, canonifyUrls, uglyUrls bool) {
+	viper.Set("CanonifyUrls", canonifyUrls)
 	viper.Set("UglyUrls", uglyUrls)
+
 	ts := setupMenuTests(t, MENU_PAGE_SOURCES)
 	defer resetMenuTestState(ts)
 
 	unicodeRussian := ts.findTestMenuEntryById("unicode", "unicode-russian")
 
-	expectedBase := "http://foo.local/zoo/%D0%BD%D0%BE%D0%B2%D0%BE%D1%81%D1%82%D0%B8-%D0%BF%D1%80%D0%BE%D0%B5%D0%BA%D1%82%D0%B0"
+	expectedBase := "/%D0%BD%D0%BE%D0%B2%D0%BE%D1%81%D1%82%D0%B8-%D0%BF%D1%80%D0%BE%D0%B5%D0%BA%D1%82%D0%B0"
+
+	if !canonifyUrls {
+		expectedBase = "/zoo" + expectedBase
+	}
+
 	var expected string
 	if uglyUrls {
 		expected = expectedBase + ".html"
@@ -288,6 +297,7 @@ func doTestMenuWithUnicodeUrls(t *testing.T, uglyUrls bool) {
 }
 
 func TestTaxonomyNodeMenu(t *testing.T) {
+	viper.Set("CanonifyUrls", true)
 	ts := setupMenuTests(t, MENU_PAGE_SOURCES)
 	defer resetMenuTestState(ts)
 
@@ -333,7 +343,7 @@ func TestHomeNodeMenu(t *testing.T) {
 	defer resetMenuTestState(ts)
 
 	home := ts.site.newHomeNode()
-	homeMenuEntry := &MenuEntry{Name: home.Title, Url: string(home.Permalink)}
+	homeMenuEntry := &MenuEntry{Name: home.Title, Url: home.Url}
 
 	for i, this := range []struct {
 		menu           string

hugolib/node.go

@@ -38,7 +38,7 @@ func (n *Node) Now() time.Time {
 
 func (n *Node) HasMenuCurrent(menuId string, inme *MenuEntry) bool {
 	if inme.HasChildren() {
-		me := MenuEntry{Name: n.Title, Url: string(n.Permalink)}
+		me := MenuEntry{Name: n.Title, Url: n.Url}
 
 		for _, child := range inme.Children {
 			if me.IsSameResource(child) {
@@ -52,8 +52,7 @@ func (n *Node) HasMenuCurrent(menuId string, inme *MenuEntry) bool {
 
 func (n *Node) IsMenuCurrent(menuId string, inme *MenuEntry) bool {
 
-	me := MenuEntry{Name: n.Title, Url: string(n.Permalink)}
-
+	me := MenuEntry{Name: n.Title, Url: n.Url}
 	if !me.IsSameResource(inme) {
 		return false
 	}

hugolib/page.go

@@ -397,6 +397,16 @@ func (p *Page) RelPermalink() (string, error) {
 		return "", err
 	}
 
+	if viper.GetBool("CanonifyUrls") {
+		// replacements for relpermalink with baseUrl on the form http://myhost.com/sub/ will fail later on
+		// have to return the Url relative from baseUrl
+		relpath, err := helpers.GetRelativePath(link.String(), string(p.Site.BaseUrl))
+		if err != nil {
+			return "", err
+		}
+		return "/" + filepath.ToSlash(relpath), nil
+	}
+
 	link.Scheme = ""
 	link.Host = ""
 	link.User = nil
@@ -549,7 +559,7 @@ func (page *Page) Menus() PageMenus {
 	ret := PageMenus{}
 
 	if ms, ok := page.Params["menu"]; ok {
-		link, _ := page.Permalink()
+		link, _ := page.RelPermalink()
 
 		me := MenuEntry{Name: page.LinkTitle(), Weight: page.Weight, Url: link}
 

hugolib/page_permalink_test.go

@@ -16,29 +16,34 @@ func TestPermalink(t *testing.T) {
 		base         template.URL
 		slug         string
 		url          string
-		uglyurls    bool
+		uglyUrls     bool
+		canonifyUrls bool
 		expectedAbs  string
 		expectedRel  string
 	}{
-		{"x/y/z/boofar.md", "x/y/z", "", "", "", false, "/x/y/z/boofar/", "/x/y/z/boofar/"},
-		{"x/y/z/boofar.md", "x/y/z/", "", "", "", false, "/x/y/z/boofar/", "/x/y/z/boofar/"},
-		{"x/y/z/boofar.md", "x/y/z/", "", "boofar", "", false, "/x/y/z/boofar/", "/x/y/z/boofar/"},
-		{"x/y/z/boofar.md", "x/y/z", "http://barnew/", "", "", false, "http://barnew/x/y/z/boofar/", "/x/y/z/boofar/"},
-		{"x/y/z/boofar.md", "x/y/z/", "http://barnew/", "boofar", "", false, "http://barnew/x/y/z/boofar/", "/x/y/z/boofar/"},
-		{"x/y/z/boofar.md", "x/y/z", "", "", "", true, "/x/y/z/boofar.html", "/x/y/z/boofar.html"},
-		{"x/y/z/boofar.md", "x/y/z/", "", "", "", true, "/x/y/z/boofar.html", "/x/y/z/boofar.html"},
-		{"x/y/z/boofar.md", "x/y/z/", "", "boofar", "", true, "/x/y/z/boofar.html", "/x/y/z/boofar.html"},
-		{"x/y/z/boofar.md", "x/y/z", "http://barnew/", "", "", true, "http://barnew/x/y/z/boofar.html", "/x/y/z/boofar.html"},
-		{"x/y/z/boofar.md", "x/y/z/", "http://barnew/", "boofar", "", true, "http://barnew/x/y/z/boofar.html", "/x/y/z/boofar.html"},
+		{"x/y/z/boofar.md", "x/y/z", "", "", "", false, false, "/x/y/z/boofar/", "/x/y/z/boofar/"},
+		{"x/y/z/boofar.md", "x/y/z/", "", "", "", false, false, "/x/y/z/boofar/", "/x/y/z/boofar/"},
+		{"x/y/z/boofar.md", "x/y/z/", "", "boofar", "", false, false, "/x/y/z/boofar/", "/x/y/z/boofar/"},
+		{"x/y/z/boofar.md", "x/y/z", "http://barnew/", "", "", false, false, "http://barnew/x/y/z/boofar/", "/x/y/z/boofar/"},
+		{"x/y/z/boofar.md", "x/y/z/", "http://barnew/", "boofar", "", false, false, "http://barnew/x/y/z/boofar/", "/x/y/z/boofar/"},
+		{"x/y/z/boofar.md", "x/y/z", "", "", "", true, false, "/x/y/z/boofar.html", "/x/y/z/boofar.html"},
+		{"x/y/z/boofar.md", "x/y/z/", "", "", "", true, false, "/x/y/z/boofar.html", "/x/y/z/boofar.html"},
+		{"x/y/z/boofar.md", "x/y/z/", "", "boofar", "", true, false, "/x/y/z/boofar.html", "/x/y/z/boofar.html"},
+		{"x/y/z/boofar.md", "x/y/z", "http://barnew/", "", "", true, false, "http://barnew/x/y/z/boofar.html", "/x/y/z/boofar.html"},
+		{"x/y/z/boofar.md", "x/y/z/", "http://barnew/", "boofar", "", true, false, "http://barnew/x/y/z/boofar.html", "/x/y/z/boofar.html"},
+		{"x/y/z/boofar.md", "x/y/z/", "http://barnew/boo/", "boofar", "", true, false, "http://barnew/boo/x/y/z/boofar.html", "/boo/x/y/z/boofar.html"},
+		{"x/y/z/boofar.md", "x/y/z/", "http://barnew/boo/", "boofar", "", true, true, "http://barnew/boo/x/y/z/boofar.html", "/x/y/z/boofar.html"},
+		{"x/y/z/boofar.md", "x/y/z/", "http://barnew/boo", "boofar", "", true, true, "http://barnew/boo/x/y/z/boofar.html", "/x/y/z/boofar.html"},
 
 		// test url overrides
-		{"x/y/z/boofar.md", "x/y/z", "", "", "/z/y/q/", false, "/z/y/q/", "/z/y/q/"},
+		{"x/y/z/boofar.md", "x/y/z", "", "", "/z/y/q/", false, false, "/z/y/q/", "/z/y/q/"},
 	}
 
 	viper.Set("DefaultExtension", "html")
 
 	for i, test := range tests {
-		viper.Set("uglyurls", test.uglyurls)
+		viper.Set("uglyurls", test.uglyUrls)
+		viper.Set("canonifyurls", test.canonifyUrls)
 		p := &Page{
 			Node: Node{
 				UrlPath: UrlPath{
@@ -75,7 +80,7 @@ func TestPermalink(t *testing.T) {
 
 		expected = test.expectedRel
 		if u != expected {
-			t.Errorf("Test %d: Expected abs url: %s, got: %s", i, expected, u)
+			t.Errorf("Test %d: Expected rel url: %s, got: %s", i, expected, u)
 		}
 	}
 }

hugolib/site.go

@@ -106,6 +106,7 @@ type SiteInfo struct {
 	Permalinks      PermalinkOverrides
 	Params          map[string]interface{}
 	BuildDrafts     bool
+	canonifyUrls    bool
 }
 
 // SiteSocial is a place to put social details on a site level. These are the
@@ -362,6 +363,7 @@ func (s *Site) initializeSiteInfo() {
 		Copyright:       viper.GetString("copyright"),
 		DisqusShortname: viper.GetString("DisqusShortname"),
 		BuildDrafts:     viper.GetBool("BuildDrafts"),
+		canonifyUrls:    viper.GetBool("CanonifyUrls"),
 		Pages:           &s.Pages,
 		Recent:          &s.Pages,
 		Menus:           &s.Menus,
@@ -608,10 +610,16 @@ func (s *Site) getMenusFromConfig() Menus {
 					}
 
 					menuEntry.MarshallMap(ime)
+
 					if strings.HasPrefix(menuEntry.Url, "/") {
-						// make it absolute so it matches the nodes
-						menuEntry.Url = s.permalinkStr(menuEntry.Url)
+						// make it match the nodes
+						menuEntryUrl := menuEntry.Url
+						if !s.Info.canonifyUrls {
+							menuEntryUrl = helpers.AddContextRoot(string(s.Info.BaseUrl), menuEntryUrl)
 						}
+						menuEntry.Url = s.prepUrl(menuEntryUrl)
+					}
+
 					if ret[name] == nil {
 						ret[name] = &Menu{}
 					}