threads / patch / 32064

patchgitweb: git_summary - show $project in title

Subject: [PATCH] gitweb: git_summary - show $project in title

## tl;dr

4 messages between Nov 11, 2012 and Nov 29, 2012. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Henrich Schuchardt· Nov 11, 2012, 05:20 UTC · lore

Gitweb pages are structured by divs of class title with grey background. The shortlog, and the log page show the project name as the first title. Page summary only shows an empty grey box above the project details. This provides an inconstent user experience.

This patch adds the missing project title.
Signed-off-by: Henrich Schuchardt <xypron.glpk@gmx.de>
---
 gitweb/gitweb.perl |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to gitweb/gitweb.perl +1 −1
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 10ed9e5..3e1c452 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -6451,7 +6451,7 @@ sub git_summary {
 	git_header_html();
 	git_print_page_nav('summary','', $head);
 
-	print "<div class=\"title\">&nbsp;</div>\n";
+	print "<div class=\"title\">$project</div>\n";
 	print "<table class=\"projects_list\">\n" .
 	      "<tr id=\"metadata_desc\"><td>description</td><td>" . esc_html($descr) . "</td></tr>\n";
         unless ($omit_owner) {
-- 
1.7.10.4
Jeff King· Nov 12, 2012, 23:25 UTC · re: Henrich Schuchardt · lore

Re: [PATCH] gitweb: git_summary - show $project in title

On Sun, Nov 11, 2012 at 06:20:58AM +0100, Henrich Schuchardt wrote:
Show 22 quoted lines
> Gitweb pages are structured by divs of class title with grey background.
> The shortlog, and the log page show the project name as the first title.
> Page summary only shows an empty grey box above the project details.
> This provides an inconstent user experience.
> 
> This patch adds the missing project title.
> 
> Signed-off-by: Henrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  gitweb/gitweb.perl |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 10ed9e5..3e1c452 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -6451,7 +6451,7 @@ sub git_summary {
>  	git_header_html();
>  	git_print_page_nav('summary','', $head);
>  
> -	print "<div class=\"title\">&nbsp;</div>\n";
> +	print "<div class=\"title\">$project</div>\n";

I do not have any opinion on whether the intent of the change is good or not, but shouldn't $project be run through esc_html() here?

-Peff
Junio C Hamano· Nov 13, 2012, 00:46 UTC · re: Jeff King · lore

Re: [PATCH] gitweb: git_summary - show $project in title

Jeff King <peff@peff.net> writes:
Show 27 quoted lines
> On Sun, Nov 11, 2012 at 06:20:58AM +0100, Henrich Schuchardt wrote:
>
>> Gitweb pages are structured by divs of class title with grey background.
>> The shortlog, and the log page show the project name as the first title.
>> Page summary only shows an empty grey box above the project details.
>> This provides an inconstent user experience.
>> 
>> This patch adds the missing project title.
>> 
>> Signed-off-by: Henrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>  gitweb/gitweb.perl |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index 10ed9e5..3e1c452 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -6451,7 +6451,7 @@ sub git_summary {
>>  	git_header_html();
>>  	git_print_page_nav('summary','', $head);
>>  
>> -	print "<div class=\"title\">&nbsp;</div>\n";
>> +	print "<div class=\"title\">$project</div>\n";
>
> I do not have any opinion on whether the intent of the change is good or
> not, but shouldn't $project be run through esc_html() here?

I think the answer is yes. And if $project needs to be escaped, the git_feed function you fixed today has another codepath that needs to be fixed. When git_get_project_description($project) returns undef, the description is taken from $project without any escaping.

Xypron· Nov 29, 2012, 21:07 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitweb: git_summary - show $project in title

Thank you for your comments. In the appended version of the patch the project title is escaped:

Subject: [PATCH] gitweb: git_summary - show $project in title

Gitweb pages are structured by divs of class title with grey background. The shortlog, and the log page show the project name as the first title. Page summary only shows an empty grey box above the project details. This provides an inconsistent user experience.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 gitweb/gitweb.perl |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to gitweb/gitweb.perl +1 −1
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index e8812fa..be94b0b 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -6450,7 +6450,7 @@ sub git_summary {
 	git_header_html();
 	git_print_page_nav('summary','', $head);
 
-	print "<div class=\"title\">&nbsp;</div>\n";
+	print "<div class=\"title\">" . esc_html($project) . "</div>\n";
 	print "<table class=\"projects_list\">\n" .
 		"<tr id=\"metadata_desc\"><td>description</td><td>" . esc_html($descr) . "</td></tr>\n";
         unless ($omit_owner) {
-- 
1.7.10.4


On 13.11.2012 01:46, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
>> On Sun, Nov 11, 2012 at 06:20:58AM +0100, Henrich Schuchardt wrote:
>>
>>> Gitweb pages are structured by divs of class title with grey background.
>>> The shortlog, and the log page show the project name as the first title.
>>> Page summary only shows an empty grey box above the project details.
>>> This provides an inconstent user experience.
>>>
>>> This patch adds the missing project title.
>>>
>>> Signed-off-by: Henrich Schuchardt <xypron.glpk@gmx.de>
>>> ---
>>>  gitweb/gitweb.perl |    2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>>> index 10ed9e5..3e1c452 100755
>>> --- a/gitweb/gitweb.perl
>>> +++ b/gitweb/gitweb.perl
>>> @@ -6451,7 +6451,7 @@ sub git_summary {
>>>  	git_header_html();
>>>  	git_print_page_nav('summary','', $head);
>>>  
>>> -	print "<div class=\"title\">&nbsp;</div>\n";
>>> +	print "<div class=\"title\">$project</div>\n";
>> I do not have any opinion on whether the intent of the change is good or
>> not, but shouldn't $project be run through esc_html() here?
> I think the answer is yes.  And if $project needs to be escaped, the
> git_feed function you fixed today has another codepath that needs to
> be fixed.  When git_get_project_description($project) returns undef,
> the description is taken from $project without any escaping.
>
>
>

← back to recent threads