Go 1.7: {{if}} branches end in different contexts #977

GiteaMirror commented

@lafriks commented on GitHub (Aug 18, 2017):

strange, I can't seem to be able to reproduce that. Not in new install, not in existing one. Built with TAGS="sqlite" make build

@lafriks commented on GitHub (Aug 18, 2017): strange, I can't seem to be able to reproduce that. Not in new install, not in existing one. Built with `TAGS="sqlite" make build`

GiteaMirror commented

@strk commented on GitHub (Aug 18, 2017):

Here I have go version go1.7.4 linux/amd64

The other specificity of my install is that I'm using an existing PostgreSQL database, with a user in it.
When cleaning cookies the error appears after login.

@strk commented on GitHub (Aug 18, 2017): Here I have go version go1.7.4 linux/amd64 The other specificity of my install is that I'm using an existing PostgreSQL database, with a user in it. When cleaning cookies the error appears *after* login.

GiteaMirror commented

@strk commented on GitHub (Aug 18, 2017):

I can still reproduce with TAGS="sqlite" make build builds (and sqlite3 database).
In this case I have to register a new user before logging in, and get that error upon first login.

@strk commented on GitHub (Aug 18, 2017): I can still reproduce with `TAGS="sqlite" make build` builds (and sqlite3 database). In this case I have to register a new user before logging in, and get that error upon first login.

GiteaMirror commented

@strk commented on GitHub (Aug 18, 2017):

The template does indeed look weird, check this snippet:

        <div v-if="tab === 'repos'" class="ui tab active list">

What's that v-if ??

@strk commented on GitHub (Aug 18, 2017): The template does indeed look weird, check this snippet: ``` <div v-if="tab === 'repos'" class="ui tab active list"> ``` What's that `v-if` ??

GiteaMirror commented

@daviian commented on GitHub (Aug 18, 2017):

@strk It's from Vue.js. Do you use Chrome? Chrome has an intriguing caching now. Open your public/index.js in a seperate tab and reload. Maybe you have an old js file.

@daviian commented on GitHub (Aug 18, 2017): @strk It's from Vue.js. Do you use Chrome? Chrome has an intriguing caching now. Open your public/index.js in a seperate tab and reload. Maybe you have an old js file.

GiteaMirror commented

@strk commented on GitHub (Aug 18, 2017):

Ok, anyway the problem is somewhere in here, at the top of templates/user/dashboard/repo-search.tmpl:

        {{if not .ContextUser.IsOrganization}}
        <div class="ui two item stackable tabable menu">
            <a :class="{item: true, active: tab === 'repos'}" @click="changeTab('repos')">{{.i18n.Tr "repository"}}</a>
            <a :class="{item: true, active: tab === 'orgs'}" @click="changeTab('orgs')">{{.i18n.Tr "organization"}}</a>
        </div>
        {{end}}

Removing the portion inside the condition, leaving the following, fixes it:

        {{if not .ContextUser.IsOrganization}}
        {{end}}

@strk commented on GitHub (Aug 18, 2017): Ok, anyway the problem is somewhere in here, at the top of templates/user/dashboard/repo-search.tmpl: ``` {{if not .ContextUser.IsOrganization}} <div class="ui two item stackable tabable menu"> <a :class="{item: true, active: tab === 'repos'}" @click="changeTab('repos')">{{.i18n.Tr "repository"}}</a> <a :class="{item: true, active: tab === 'orgs'}" @click="changeTab('orgs')">{{.i18n.Tr "organization"}}</a> </div> {{end}} ``` Removing the portion inside the condition, leaving the following, fixes it: ``` {{if not .ContextUser.IsOrganization}} {{end}} ```

GiteaMirror commented

@daviian commented on GitHub (Aug 18, 2017):

I can't reproduce that error as well. I am on latest master. My Go version is 1.8.3

@daviian commented on GitHub (Aug 18, 2017): I can't reproduce that error as well. I am on latest master. My Go version is 1.8.3

GiteaMirror commented

@strk commented on GitHub (Aug 18, 2017):

What Go version are you using ? What else could be different ?

@strk commented on GitHub (Aug 18, 2017): What Go version are you using ? What else could be different ?

GiteaMirror commented

@daviian commented on GitHub (Aug 18, 2017):

As I said, Go version is 1.8.3
Have you tried hard resetting your git repository to remove old folders etc.? Just an idea.

@daviian commented on GitHub (Aug 18, 2017): As I said, Go version is 1.8.3 Have you tried hard resetting your git repository to remove old folders etc.? Just an idea.

GiteaMirror commented

@strk commented on GitHub (Aug 18, 2017):

@lafriks are you also using Go 1.8 ? Both: any chance you can try an older version ?
I'll repeat the steps to reproduce:

Install Gitea (tried both PostgreSQL and Sqlite3 backends)
Register new account (will become admin)
Login as the new account

@strk commented on GitHub (Aug 18, 2017): @lafriks are you also using Go 1.8 ? Both: any chance you can try an older version ? I'll repeat the steps to reproduce: 1. Install Gitea (tried both PostgreSQL and Sqlite3 backends) 2. Register new account (will become admin) 3. Login as the new account

GiteaMirror commented

@daviian commented on GitHub (Aug 18, 2017):

@strk I will try with an older version, give me a moment.

@daviian commented on GitHub (Aug 18, 2017): @strk I will try with an older version, give me a moment.

GiteaMirror commented

@daviian commented on GitHub (Aug 18, 2017):

Ok I can confirm this issue with Go 1.7.4!

@daviian commented on GitHub (Aug 18, 2017): Ok I can confirm this issue with Go 1.7.4!

GiteaMirror commented

@strk commented on GitHub (Aug 18, 2017):

Thanks for testing ! Should this be filed upstream to Vue ? (or can you tell where else the bug is ?)

@strk commented on GitHub (Aug 18, 2017): Thanks for testing ! Should this be filed upstream to Vue ? (or can you tell where else the bug is ?)

GiteaMirror commented

@daviian commented on GitHub (Aug 18, 2017):

I don't think Vue is responsible for this. I'd rather say to file an issue at Golang for the bug in 1.7 ;-)

@daviian commented on GitHub (Aug 18, 2017): I don't think Vue is responsible for this. I'd rather say to file an issue at Golang for the bug in 1.7 ;-)

GiteaMirror commented

@strk commented on GitHub (Aug 18, 2017):

From https://golang.org/pkg/html/template/:

        // ErrBranchEnd: "{{if}} branches end in different contexts"
        // Example:
        //   {{if .C}}<a href="{{end}}{{.X}}
        // Discussion:
        //   Package html/template statically examines each path through an
        //   {{if}}, {{range}}, or {{with}} to escape any following pipelines.
        //   The example is ambiguous since {{.X}} might be an HTML text node,
        //   or a URL prefix in an HTML attribute. The context of {{.X}} is
        //   used to figure out how to escape it, but that context depends on
        //   the run-time value of {{.C}} which is not statically known.
        //
        //   The problem is usually something like missing quotes or angle
        //   brackets, or can be avoided by refactoring to put the two contexts
        //   into different branches of an if, range or with. If the problem
        //   is in a {{range}} over a collection that should never be empty,
        //   adding a dummy {{else}} can help.
        ErrBranchEnd

Maybe we can try the dummy {{else}} trick...

@strk commented on GitHub (Aug 18, 2017): From https://golang.org/pkg/html/template/: ``` // ErrBranchEnd: "{{if}} branches end in different contexts" // Example: // {{if .C}}<a href="{{end}}{{.X}} // Discussion: // Package html/template statically examines each path through an // {{if}}, {{range}}, or {{with}} to escape any following pipelines. // The example is ambiguous since {{.X}} might be an HTML text node, // or a URL prefix in an HTML attribute. The context of {{.X}} is // used to figure out how to escape it, but that context depends on // the run-time value of {{.C}} which is not statically known. // // The problem is usually something like missing quotes or angle // brackets, or can be avoided by refactoring to put the two contexts // into different branches of an if, range or with. If the problem // is in a {{range}} over a collection that should never be empty, // adding a dummy {{else}} can help. ErrBranchEnd ``` Maybe we can try the dummy `{{else}}` trick...

GiteaMirror commented

@strk commented on GitHub (Aug 18, 2017):

dummy {{else}} does not help :(

@strk commented on GitHub (Aug 18, 2017): dummy `{{else}}` does not help :(

GiteaMirror commented

@daviian commented on GitHub (Aug 18, 2017):

I found this excerpt earlier today too, but thought that won't help in any way ;-)

Why not just updating Go?

@daviian commented on GitHub (Aug 18, 2017): I found this excerpt earlier today too, but thought that won't help in any way ;-) Why not just updating Go?

GiteaMirror commented

@daviian commented on GitHub (Aug 18, 2017):

There was a major change in text/template from Go 1.7.5 to 1.8.0

@daviian commented on GitHub (Aug 18, 2017): There was a major change in text/template from Go 1.7.5 to 1.8.0

GiteaMirror commented

@strk commented on GitHub (Aug 18, 2017):

Go 1.7 is what is shipped by latest Long-Term-Support popular free unix distributions.
If we drop support for 1.7 it must be properly documented (and decided).
Me, I'd rather find a workaround than dropping support for 1.7.
Check out CHANGELOG.md, the BREAKING section is where a drop of support for 1.7 would go.

@strk commented on GitHub (Aug 18, 2017): Go 1.7 is what is shipped by latest Long-Term-Support popular free unix distributions. If we drop support for 1.7 it must be properly documented (and decided). Me, I'd rather find a workaround than dropping support for 1.7. Check out CHANGELOG.md, the `BREAKING` section is where a drop of support for 1.7 would go.

GiteaMirror commented

@daviian commented on GitHub (Aug 18, 2017):

Ok sorry, haven't thought about that.

@daviian commented on GitHub (Aug 18, 2017): Ok sorry, haven't thought about that.

GiteaMirror commented

@andyleap commented on GitHub (Aug 18, 2017):

Most likely the issue is ffd1c781b7

As the file in question is all wrapped in a <script>, 1.7.4 and before are treating it as JS, but 1.8 sees that it's not JS. Not sure how much you can do to fix it.

@andyleap commented on GitHub (Aug 18, 2017): Most likely the issue is https://github.com/golang/go/commit/ffd1c781b77aab542713b66ef387fa9307e4060b As the file in question is all wrapped in a <script>, 1.7.4 and before are treating it as JS, but 1.8 sees that it's not JS. Not sure how much you can do to fix it.

GiteaMirror commented

@strk commented on GitHub (Aug 18, 2017):

Dropping the <script type="text/x-template" id="repo-search-template"> and relative closing tag prevents the error message, but doesn't give a correct rendering. Turning the type to text/template doesn't prevent the error message.

I'll note that this template is the only template using that <script> hack (which is documented elsewhere to be an hack). It was introduced with PR #2285

@strk commented on GitHub (Aug 18, 2017): Dropping the `<script type="text/x-template" id="repo-search-template">` and relative closing tag prevents the error message, but doesn't give a correct rendering. Turning the type to `text/template` doesn't prevent the error message. I'll note that this template is the *only* template using that `<script>` hack (which is documented elsewhere to be an hack). It was introduced with PR #2285

GiteaMirror commented

@daviian commented on GitHub (Aug 18, 2017):

I think the script tag exists because it is a Vue template and not a Go html/template.

@daviian commented on GitHub (Aug 18, 2017): I think the script tag exists because it is a Vue template and not a Go html/template.

GiteaMirror commented

@strk commented on GitHub (Aug 18, 2017):

According to https://sebastiandedeyne.com/posts/2016/dealing-with-templates-in-vue-20 even documentation discourage from using the x-template trick.

@strk commented on GitHub (Aug 18, 2017): According to https://sebastiandedeyne.com/posts/2016/dealing-with-templates-in-vue-20 even documentation discourage from using the `x-template` trick.

GiteaMirror commented

@strk commented on GitHub (Aug 18, 2017):

Comment from #go-nuts IRC channel:

21:40 < Vendan> strk, it's def. a go template as well as a vue template
21:40 < Vendan> which is kinda an abomination, imo

@strk commented on GitHub (Aug 18, 2017): Comment from #go-nuts IRC channel: ``` 21:40 < Vendan> strk, it's def. a go template as well as a vue template 21:40 < Vendan> which is kinda an abomination, imo ```

GiteaMirror commented

@lafriks commented on GitHub (Aug 18, 2017):

@strk yes it is not best practice but currently it would be quite big task to move template out to Vue files/templates as it would need to create way to get translations to javascript + add webpack javascript build step. What could be done currently to support do not use Vue component but directly use Vue app for this. This would fix the go 1.7 problem

@lafriks commented on GitHub (Aug 18, 2017): @strk yes it is not best practice but currently it would be quite big task to move template out to Vue files/templates as it would need to create way to get translations to javascript + add webpack javascript build step. What could be done currently to support do not use Vue component but directly use Vue app for this. This would fix the go 1.7 problem

GiteaMirror commented

@lafriks commented on GitHub (Aug 18, 2017):

@Morlinest can you for now make that vue component back to vue app so not to use script template so that Gitea can be compiled with GoLang 1.7 again? After 1.2 is released we should make correct solution with component moved to vue file and webpack build step

@lafriks commented on GitHub (Aug 18, 2017): @Morlinest can you for now make that vue component back to vue app so not to use script template so that Gitea can be compiled with GoLang 1.7 again? After 1.2 is released we should make correct solution with component moved to vue file and webpack build step

GiteaMirror commented

@Morlinest commented on GitHub (Aug 18, 2017):

@strk @lafriks Interesting ... I can make some changes, for example use inline-template instead (or back to Vue). Can you try first if #2317 can fix it? Did you try it before x-template was used?

@Morlinest commented on GitHub (Aug 18, 2017): @strk @lafriks Interesting ... I can make some changes, for example use `inline-template` instead (or back to `Vue`). Can you try first if #2317 can fix it? Did you try it before `x-template` was used?

GiteaMirror commented

@Morlinest commented on GitHub (Aug 18, 2017):

If #2317 does not help, aslo try to include code from repo-search.tmpl directly to dashboard.tmpl.

@Morlinest commented on GitHub (Aug 18, 2017): If #2317 does not help, aslo try to include code from `repo-search.tmpl` directly to `dashboard.tmpl`.

GiteaMirror commented

@daviian commented on GitHub (Aug 18, 2017):

@Morlinest The problem is the <script>-tag. As it tells the templating engine of Go 1.7 to interpret this part as JS. Mentioned in https://github.com/go-gitea/gitea/issues/2331#issuecomment-323440737
Don't think copying the <script> part directly to dashboard.tmpl resolves the issue.

@daviian commented on GitHub (Aug 18, 2017): @Morlinest The problem is the <script>-tag. As it tells the templating engine of Go 1.7 to interpret this part as JS. Mentioned in https://github.com/go-gitea/gitea/issues/2331#issuecomment-323440737 Don't think copying the <script> part directly to dashboard.tmpl resolves the issue.

GiteaMirror commented

@strk commented on GitHub (Aug 18, 2017):

#2317 seems to fix it for me. Do you confirm @daviian ?

@strk commented on GitHub (Aug 18, 2017): #2317 seems to fix it for me. Do you confirm @daviian ?

GiteaMirror commented

@lafriks commented on GitHub (Aug 18, 2017):

@strk it will fix template compile error but will not fix rendering on 1.7 as it will escape translations as if they are part of javascript that is not correct.

@lafriks commented on GitHub (Aug 18, 2017): @strk it will fix template compile error but will not fix rendering on 1.7 as it will escape translations as if they are part of javascript that is not correct.

GiteaMirror commented

@lafriks commented on GitHub (Aug 18, 2017):

@Morlinest I think currently it is needed to change it back to Vue app

@lafriks commented on GitHub (Aug 18, 2017): @Morlinest I think currently it is needed to change it back to Vue app

GiteaMirror commented

@daviian commented on GitHub (Aug 18, 2017):

@strk Yes I can confirm, however there are lots of errors.

imo @lafriks is right. The best solution would be a complete webpack build chain and a clean separation between Vue templates and Go templates. I guess this is not doable for 1.2.0

@daviian commented on GitHub (Aug 18, 2017): @strk Yes I can confirm, however there are lots of errors. ![repo-search-errors](https://user-images.githubusercontent.com/5477981/29479251-5a07cc7a-8471-11e7-9e27-e672abfcb56b.PNG) imo @lafriks is right. The best solution would be a complete webpack build chain and a clean separation between Vue templates and Go templates. I guess this is not doable for 1.2.0

GiteaMirror commented

@Morlinest commented on GitHub (Aug 18, 2017):

OK, just <script> tag is no no. I'll do something with it.

@Morlinest commented on GitHub (Aug 18, 2017): OK, just `<script>` tag is no no. I'll do something with it.

GiteaMirror commented

@Morlinest commented on GitHub (Aug 18, 2017):

Please try #2336

@Morlinest commented on GitHub (Aug 18, 2017): Please try #2336

GiteaMirror commented