Versioncontrol browser: "export:" links formatted incorrectly

[David Avsajanishvili <avsd05@…>]

As a result of changeset:9125/trunk/trac/versioncontrol/web_ui/browser.py the "export:" browser link does not work correctly anymore, because following line has been changed:

source:trunk/trac/versioncontrol/web_ui/browser.py@9125#L727​ _format_export_link() method:

(a) old vs. (b) new

@@ -621 +725 @@
             path, rev = export.split('@', 1)
         else:
-            rev, path = self.env.get_repository().youngest_rev, export
+            rev, path = '', export
         return tag.a(label, class_='export',
                      href=formatter.href.export(rev, path) + fragment)

As a result, following kind of links are formed:

​http://trac.edgewall.org/export//trunk/README

I have fixed this issue in my environment by returning old line back, and it worked! No other problems appeared!

Just don't understand, why this line was changed…

[Remy Blank]

This looks weird indeed. Thanks for reporting the issue and fix.

[Remy Blank]

For reference, here's a non-working link: export:trunk/README

The links seem to work with non-default repositories. More investigation needed.

[Christian Boos]

The rev as an empty string '' is supposed to be an alias for youngest rev (see Repository.normalize_rev), and it's used that way in BrowserModule.process_request, so in theory, it should work.

In my test environment served by tracd, export links like export:trunk/README work.

Here on t.e.o, lighty sees the correct URL: "GET /export//trunk/README HTTP/1.1", but Trac not, it gets path_info: u'/export/trunk/README', hence path @ rev becomes u'trunk' @ u'README'.

So it looks like we can't rely on using '//' in URLs, as some web front ends seem to normalize them into a single '/'.

We could instead set rev to some special value indicative of "latest". Maybe standardize on HEAD? ("latest" itself could be a branch name in DVCS, so not a good idea).

[Christian Boos]

(and URL encoding // would be also be fragile, see ​http://httpd.apache.org/docs/2.2/mod/core.html#allowencodedslashes).

[David Avsajanishvili <avsd05@…>]

In my installation these "double-slash links" also work, but IT IS CACHED BY BROWSER! Once a revision of file is changed, the web-browser still shows old one from cache, without any notification. I couldn't find a way to get correct link on the last revision, that is not affected by caching, unless manually typing the revision number between the slashes.

So, the old approach, when revision number was automatically placed in URL explicitly, probably, is better. Or, at least, this behaviour should be configurable.

Replying to cboos:

The rev as an empty string '' is supposed to be an alias for youngest rev (see Repository.normalize_rev), and it's used that way in BrowserModule.process_request, so in theory, it should work.

In my test environment served by tracd, export links like export:trunk/README work.

Here on t.e.o, lighty sees the correct URL: "GET /export//trunk/README HTTP/1.1", but Trac not, it gets path_info: u'/export/trunk/README', hence path @ rev becomes u'trunk' @ u'README'.

So it looks like we can't rely on using '//' in URLs, as some web front ends seem to normalize them into a single '/'.

We could instead set rev to some special value indicative of "latest". Maybe standardize on HEAD? ("latest" itself could be a branch name in DVCS, so not a good idea).

[Christian Boos]

Replying to David Avsajanishvili <avsd05@…>:

I couldn't

find a way to get correct link on the last revision, that is not affected by caching

Correct, if we specify HEAD as the revision, then the browser shouldn't cache the page and should always try to get the content for the latest revision.

So, the old approach, when revision number was automatically placed in URL explicitly, probably, is better.

I don't see how… if we output the revision number, then it is appropriate to make the browser cache it, as you'll be trying to retrieve the file at that same revision again, hence with the same content.

[David Avsajanishvili <avsd05@…>]

In fact, the only problem is that browser always tries to cache the export link. So, once I have clicked "Download in other formats → Original format" link when browsing the source, the Web-browser downloads the file and CACHES IT! If I submit some changes of this in SVN, and return to source browsing page again and click on "Original format" link, the Web-browser shows me cached version of the file, rather than downloading fresh one from the server!

I use Trac to store PDF files and view them directly by the web-browser, by setting render_unsafe_content to True. So, in Trac-dev012 my users often get unexpected cached version of PDF file, instead of expecting fresh one. That's why I had to rollback this line of code in my Trac installation.

If you could help avoiding this caching - it would be nice!

[Christian Boos]

To sum up:

• normalize on 'HEAD' meaning youngest rev
• ensure we send the appropriate Cache-Control headers

[Remy Blank]

Implemented in [9588].

[Remy Blank]

OT: Weird, any changes to this ticket seem to trigger the message "The ticket validation has failed", without there being any warnings at the top of the page. Other tickets don't show this issue. I wonder where this comes from…

[Christian Boos]

Previewing a change, either as anonymous or when logged in, triggers a validation error.

Looking in the Trac log, I see:

[pid  1207519568] 2010-04-29 07:01:58,624 Trac[api] WARNING: HTML preview using <trac.mimeview.patch.PatchRenderer object at 0x7fc5fc09f6d0> failed (
Traceback (most recent call last):
  File "build/bdist.linux-x86_64/egg/trac/mimeview/api.py", line 761, in render
    rendered_content, filename, url)
  File "build/bdist.linux-x86_64/egg/trac/mimeview/patch.py", line 54, in render
    raise TracError(_('Invalid unified diff content'))
TracError: Le contenu n'est pas conforme au format diff unifié)

[Christian Boos]

Replying to cboos:

Previewing a change, …

Saving obviously worked ;-)

After "fixing" the patch in the description, previewing works as well.