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

Jeff King [off-list ref] writes:

On Sun, Nov 11, 2012 at 06:20:58AM +0100, Henrich Schuchardt wrote:

quoted

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 <redacted>
---
 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.

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 <redacted>
---
 gitweb/gitweb.perl |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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.
>
>
>