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

8 files changed, 112 insertions, 31 deletions

diff --git a/docs/content/extras/menus.md b/docs/content/extras/menus.md
index 957432567..fcf90096f 100644
--- a/docs/content/extras/menus.md
+++ b/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.
diff --git a/helpers/url.go b/helpers/url.go
index 46bc951f4..1b7608178 100644
--- a/helpers/url.go
+++ b/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))
diff --git a/helpers/url_test.go b/helpers/url_test.go
index 1d5c770ea..3df1a05c2 100644
--- a/helpers/url_test.go
+++ b/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")
diff --git a/hugolib/menu_test.go b/hugolib/menu_test.go
index afe881a94..667105f50 100644
--- a/hugolib/menu_test.go
+++ b/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
diff --git a/hugolib/node.go b/hugolib/node.go
index 0c3ed9ce6..ccf3e8822 100644
--- a/hugolib/node.go
+++ b/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
 	}
diff --git a/hugolib/page.go b/hugolib/page.go
index c30728c03..395462d84 100644
--- a/hugolib/page.go
+++ b/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}
 
diff --git a/hugolib/page_permalink_test.go b/hugolib/page_permalink_test.go
index 97b3d24c6..b73e08721 100644
--- a/hugolib/page_permalink_test.go
+++ b/hugolib/page_permalink_test.go
@@ -11,34 +11,39 @@ import (
 
 func TestPermalink(t *testing.T) {
 	tests := []struct {
-		file        string
-		dir         string
-		base        template.URL
-		slug        string
-		url         string
-		uglyurls    bool
-		expectedAbs string
-		expectedRel string
+		file         string
+		dir          string
+		base         template.URL
+		slug         string
+		url          string
+		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)
 		}
 	}
 }
diff --git a/hugolib/site.go b/hugolib/site.go
index 413e9f4e2..95a978f93 100644
--- a/hugolib/site.go
+++ b/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{}
 					}