Re: [PATCHv4 1/2] gitweb: make static files accessible with PATH_INFO
On Wed, 28 Jan 2009, Giuseppe Bilotta wrote:
Show 8 quoted lines
> When gitweb is invoked with PATH_INFO and links to static files such as
> the CSS and favicon/shortcut icon are relative URLs with relative paths
> (as is the case when using the default Makefile), these files are not
> accessible beyond the project list and summary page (e.g. in shortlog or
> commit view).
>
> Fix this by adding a <base> tag pointing to the script's own URL, that
> ensure that all relative paths will be based on this.
I think it is a very good idea, good patch (now with esc_url, just in case, even though I think it should be needed), and commit message has all info that it should have. But I think that it could be phrased better: instead of one long sentence, perhaps split it into few sentences, each dealing with one issue. Something like below:
-- >8 -- Gitweb links to a few static files: CSS stylesheet, favicon/shortcut icon and git logo. When links to those files are given by relative URLs with relative paths (not starting with '/'), and gitweb is invoked with (non empty) PATH_INFO, these files are not accessible beyond projects list and 'summary' view for a project (e.g. in 'shortlog' or 'commit' view). Default Makefile rules use base filenames for those static files.
Fix this by adding <base> element pointing to script's own URL, which ensures that all relative paths relative URLs will be resolved correctly. -- 8< --
Gaaahhh... I think this version could be better, but I cannot think how it should look like...
Show 16 quoted lines
>
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
> ---
> gitweb/gitweb.perl | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 931db4f..f7aaf9a 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -2901,6 +2901,11 @@ sub git_header_html {
> <meta name="robots" content="index, nofollow"/>
> <title>$title</title>
> EOF
> +# the stylesheet, favicon etc urls won't work correctly with path_info unless we
> +# set the appropriate base URLBTW. I think there should be independent patch making those comments indented properly...
> + if ($ENV{'PATH_INFO'}) {
> + print "<base href='".esc_url($my_url)."' />\n";Errr... here we use ' as attribute delimiter, while everywhere else we use "; I don't know if esc_url deals correctly with this... it does as neither " nor ' is in "allowed" list, but I'd personally use
+ print '<base href="'.esc_url($my_url).'" />'."\n";
Show 8 quoted lines
> + }
> # print out each stylesheet that exist
> if (defined $stylesheet) {
> #provides backwards capability for those people who define style sheet in a config file
> --
> 1.5.6.5
>
> --
Jakub Narebski
Poland
Giuseppe Bilotta