git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [BUG/RFH] gitweb: Trouble with ref markers being hyperlinks because of illegally nested links
@ 2009-01-12  1:15 Jakub Narebski
  2009-01-12  2:59 ` Giuseppe Bilotta
  0 siblings, 1 reply; 8+ messages in thread
From: Jakub Narebski @ 2009-01-12  1:15 UTC (permalink / raw)
  To: git; +Cc: Giuseppe Bilotta

Commit 4afbaef by Giuseppe Bilotta (gitweb: ref markers link to named 
shortlogs) turned ref markers for tags and heads into links to    
appropriate views for the ref name.

Unfortunately the code didn't take into account the fact that nesting 
links (A elements) is illegal in (X)HTML:

  12.2.2 Nested links are illegal

  Links and anchors defined by the A element must not be nested;
  an A element must not contain any other A elements.

(from http://www.w3.org/TR/html401/struct/links.html#h-12.2.2), and that 
some browsers (e.g. Mozilla 1.17.2 I still use) in the very strict 
conformance mode (application/xhtml+xml mimetype and XML + XHTML DTD) 
_enforce_ this requirement by moving inner link immediately outside the 
end of outer link, i.e. for the HTML source looking like the following
  <a ...> some text <a ...>v1.5.1</a></a>
rendered HTML (which you can see using "View Selection Source") is 
instead
  <a ...> some text </a><a ...>v1.5.1</a>
And of course SPAN elements which wraps inner link (inner A element) is 
_not_ moved.


This is quite easy to fix for hyperlinked ref markers in 'shortlog' and 
'history' views: just close the "title" hyperlink before printing 
$extra, i.e. ref markers. I have even made a patch doing that. Then 
instead of incorrect
  _Merge branch into maint_ [] _maint_
where _aaa_ means that 'aaa' is hyperlink, and [xxx] is a fer marker,
we will have correct:
  _Merge branch into maint_ [_maint_]
See that we have two separate and not nested links...


What is more complicated is the issue of ref marker from 
git_print_header_div e.g. in 'commit'/'commitdiff' view, and in 'log' 
view.  There link is made into block element using "display: block;"
CSS rule (div.title, a.title), so that you can click _anywhere_ on the 
header block.  This breaks layout even worse, making hyperlinked ref 
marker text appear *below* header div:

  -----------------------------------------------------------
  |_Merge branch into maint_ []                             |
  -----------------------------------------------------------
  _maint_

To preserve current layout and behavior it would be needed to do some 
deep HTML + CSS positioning hackery, perhaps with additional link block 
without any text... But I don't know exactly how to do this; all [few] 
experiments I did failed.

I see possible the following alternate solutions:
 * Ignore this issue (e.g. if it does not affect modern browsers)
 * Revert 4afbaef (we lose feature, but how often used is it?)
 * Always use quirks mode, or check browser and use quirks mode if it
   would break layout
 * Use extra divs and links and CSS positioning to make layout which
   looks like current one, and behaves as current one, but is more
   complicated.


P.S. From what I have checked neither kernel.org nor repo.or.cz
have this issue.
-- 
Jakub Narebski
Poland

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [BUG/RFH] gitweb: Trouble with ref markers being hyperlinks because of illegally nested links
  2009-01-12  1:15 [BUG/RFH] gitweb: Trouble with ref markers being hyperlinks because of illegally nested links Jakub Narebski
@ 2009-01-12  2:59 ` Giuseppe Bilotta
  2009-01-13  0:13   ` Jakub Narebski
  0 siblings, 1 reply; 8+ messages in thread
From: Giuseppe Bilotta @ 2009-01-12  2:59 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git

On Sun, Jan 11, 2009 at 8:15 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> Commit 4afbaef by Giuseppe Bilotta (gitweb: ref markers link to named
> shortlogs) turned ref markers for tags and heads into links to
> appropriate views for the ref name.
>
> Unfortunately the code didn't take into account the fact that nesting
> links (A elements) is illegal in (X)HTML:
>
>  12.2.2 Nested links are illegal
>
>  Links and anchors defined by the A element must not be nested;
>  an A element must not contain any other A elements.
>
> (from http://www.w3.org/TR/html401/struct/links.html#h-12.2.2), and that
> some browsers (e.g. Mozilla 1.17.2 I still use) in the very strict
> conformance mode (application/xhtml+xml mimetype and XML + XHTML DTD)
> _enforce_ this requirement by moving inner link immediately outside the
> end of outer link, i.e. for the HTML source looking like the following
>  <a ...> some text <a ...>v1.5.1</a></a>
> rendered HTML (which you can see using "View Selection Source") is
> instead
>  <a ...> some text </a><a ...>v1.5.1</a>
> And of course SPAN elements which wraps inner link (inner A element) is
> _not_ moved.
>
>
> This is quite easy to fix for hyperlinked ref markers in 'shortlog' and
> 'history' views: just close the "title" hyperlink before printing
> $extra, i.e. ref markers. I have even made a patch doing that. Then
> instead of incorrect
>  _Merge branch into maint_ [] _maint_
> where _aaa_ means that 'aaa' is hyperlink, and [xxx] is a fer marker,
> we will have correct:
>  _Merge branch into maint_ [_maint_]
> See that we have two separate and not nested links...

I've seen from the list that you already have patches ready to fix at
least this problem. I think we might start from having these patches
in while we think of the best way to fix the biggest issue:

> What is more complicated is the issue of ref marker from
> git_print_header_div e.g. in 'commit'/'commitdiff' view, and in 'log'
> view.  There link is made into block element using "display: block;"
> CSS rule (div.title, a.title), so that you can click _anywhere_ on the
> header block.  This breaks layout even worse, making hyperlinked ref
> marker text appear *below* header div:
>
>  -----------------------------------------------------------
>  |_Merge branch into maint_ []                             |
>  -----------------------------------------------------------
>  _maint_
>
> To preserve current layout and behavior it would be needed to do some
> deep HTML + CSS positioning hackery, perhaps with additional link block
> without any text... But I don't know exactly how to do this; all [few]
> experiments I did failed.
>
> I see possible the following alternate solutions:
>  * Ignore this issue (e.g. if it does not affect modern browsers)

That would be my current choice until we find a better solution.

>  * Revert 4afbaef (we lose feature, but how often used is it?)
>  * Always use quirks mode, or check browser and use quirks mode if it
>   would break layout
>  * Use extra divs and links and CSS positioning to make layout which
>   looks like current one, and behaves as current one, but is more
>   complicated.

I'm asking on #html, hopefully I'll get some interesting idea to try for this.

-- 
Giuseppe "Oblomov" Bilotta

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [BUG/RFH] gitweb: Trouble with ref markers being hyperlinks because of illegally nested links
  2009-01-12  2:59 ` Giuseppe Bilotta
@ 2009-01-13  0:13   ` Jakub Narebski
  2009-01-13  0:59     ` Giuseppe Bilotta
  0 siblings, 1 reply; 8+ messages in thread
From: Jakub Narebski @ 2009-01-13  0:13 UTC (permalink / raw)
  To: Giuseppe Bilotta; +Cc: git

On Mon, 12 Jan 2009, Giuseppe Bilotta wrote:
> On Sun, Jan 11, 2009 at 8:15 PM, Jakub Narebski <jnareb@gmail.com> wrote:

> > Commit 4afbaef by Giuseppe Bilotta (gitweb: ref markers link to named
> > shortlogs) turned ref markers for tags and heads into links to
> > appropriate views for the ref name.
> >
> > Unfortunately the code didn't take into account the fact that nesting
> > links (A elements) is illegal in (X)HTML:
> >
> >  12.2.2 Nested links are illegal
> >
> >  Links and anchors defined by the A element must not be nested;
> >  an A element must not contain any other A elements.
[...]

> > What is more complicated is the issue of ref marker from
> > git_print_header_div e.g. in 'commit'/'commitdiff' view, and in 'log'
> > view.  There link is made into block element using "display: block;"
> > CSS rule (div.title, a.title), so that you can click _anywhere_ on the
> > header block.  This breaks layout even worse, making hyperlinked ref
> > marker text appear *below* header div:
> >
> >  -----------------------------------------------------------
> >  |_Merge branch into maint_ []                             |
> >  -----------------------------------------------------------
> >  _maint_
> >
> > To preserve current layout and behavior it would be needed to do some
> > deep HTML + CSS positioning hackery, perhaps with additional link block
> > without any text... But I don't know exactly how to do this; all [few]
> > experiments I did failed.
> >
> > I see possible the following alternate solutions:
> >  * Ignore this issue (e.g. if it does not affect modern browsers)
> 
> That would be my current choice until we find a better solution.

By the way, how common this error is? Could you check if _your_ web
browser (Firefox, Internet Explorer, Opera, Konqueror, Safari, Chrome)
does show this bug or not, please?

> >  * Revert 4afbaef (we lose feature, but how often used is it?)
> >  * Always use quirks mode, or check browser and use quirks mode if it
> >    would break layout
> >  * Use extra divs and links and CSS positioning to make layout which
> >    looks like current one, and behaves as current one, but is more
> >    complicated.
> 
> I'm asking on #html, hopefully I'll get some interesting idea to try for this.

I have found _a_ solution. Perhaps not the best one, but it works.
And IMHO gives / can give even better visual.

Current version (line wrapped for better visibility):
  <div class="header">
  <a class="title" href="...">GIT 1.6.1
  <span class="refs"> 
    <span class="tag indirect" title="tags/v1.6.1">
    <a href="...">v1.6.1</a>
    </span>
  </span>
  </a>
  </div>

Current CSS (relevant part):
  a.title {
  	display: block;
  	padding: 6px 8px;
  }

Current rendering:
  -----------------------------------------------------------
  |_GIT 1.6.1_ []                                           |
  -----------------------------------------------------------
  __v1.6.1__


Proposed code (line wrapped for better visibility, with CSS embedded,
which would change in final version of course). Only parts of style
related to positioning are shown.
  <div class="header">
  <a href="..." style="float: left; margin: 6px 1px 6px 8px;">GIT 1.6.1</a>
  <div style="float: left; margin: 6px 1px;">
    <span class="refs"> 
      <span class="tag indirect" title="tags/v1.6.1">
      <a href="...">v1.6.1</a>
      </span>
    </span>
  </div>
  <a href="..." style="display: block; padding: 6px 8px;">&nbsp;</a>
  </div>

Rendering with proposed code:
  -----------------------------------------------------------
  _|_GIT 1.6.1_ [_v1.6.1_]                                 |_
  -----------------------------------------------------------

I guess that instead of additional DIV element, we could use DIV for
.refs, or use "float: right" style with SPAN element. I have not
checked if other variations works: this one does.

What do you think?
-- 
Jakub Narebski
Poland

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [BUG/RFH] gitweb: Trouble with ref markers being hyperlinks because of illegally nested links
  2009-01-13  0:13   ` Jakub Narebski
@ 2009-01-13  0:59     ` Giuseppe Bilotta
  2009-01-14  0:17       ` [RFC/PATCH] gitweb: Fix nested links problem with ref markers Jakub Narebski
  0 siblings, 1 reply; 8+ messages in thread
From: Giuseppe Bilotta @ 2009-01-13  0:59 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git

On Mon, Jan 12, 2009 at 7:13 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> On Mon, 12 Jan 2009, Giuseppe Bilotta wrote:
>> On Sun, Jan 11, 2009 at 8:15 PM, Jakub Narebski <jnareb@gmail.com> wrote:
>
>> > I see possible the following alternate solutions:
>> >  * Ignore this issue (e.g. if it does not affect modern browsers)
>>
>> That would be my current choice until we find a better solution.
>
> By the way, how common this error is? Could you check if _your_ web
> browser (Firefox, Internet Explorer, Opera, Konqueror, Safari, Chrome)
> does show this bug or not, please?

Opera works fine (no display or functionality anomaly). That makes
sense, since I was the one who submitted the patch 8-D. Konqueror
3.5.9 does the ugly thing instead.

Notice that nested links are actually valid *XML*. Indeed, I asked on
www-style and they suggested leaving the problem as-is, serving as
html+xml which is what we do.

>> >  * Revert 4afbaef (we lose feature, but how often used is it?)
>> >  * Always use quirks mode, or check browser and use quirks mode if it
>> >    would break layout
>> >  * Use extra divs and links and CSS positioning to make layout which
>> >    looks like current one, and behaves as current one, but is more
>> >    complicated.
>>
>> I'm asking on #html, hopefully I'll get some interesting idea to try for this.
>
> I have found _a_ solution. Perhaps not the best one, but it works.
> And IMHO gives / can give even better visual.
>
> Current version (line wrapped for better visibility):
>  <div class="header">
>  <a class="title" href="...">GIT 1.6.1
>  <span class="refs">
>    <span class="tag indirect" title="tags/v1.6.1">
>    <a href="...">v1.6.1</a>
>    </span>
>  </span>
>  </a>
>  </div>
>
> Current CSS (relevant part):
>  a.title {
>        display: block;
>        padding: 6px 8px;
>  }
>
> Current rendering:
>  -----------------------------------------------------------
>  |_GIT 1.6.1_ []                                           |
>  -----------------------------------------------------------
>  __v1.6.1__
>
>
> Proposed code (line wrapped for better visibility, with CSS embedded,
> which would change in final version of course). Only parts of style
> related to positioning are shown.
>  <div class="header">
>  <a href="..." style="float: left; margin: 6px 1px 6px 8px;">GIT 1.6.1</a>
>  <div style="float: left; margin: 6px 1px;">
>    <span class="refs">
>      <span class="tag indirect" title="tags/v1.6.1">
>      <a href="...">v1.6.1</a>
>      </span>
>    </span>
>  </div>
>  <a href="..." style="display: block; padding: 6px 8px;">&nbsp;</a>
>  </div>
>
> Rendering with proposed code:
>  -----------------------------------------------------------
>  _|_GIT 1.6.1_ [_v1.6.1_]                                 |_
>  -----------------------------------------------------------
>
> I guess that instead of additional DIV element, we could use DIV for
> .refs, or use "float: right" style with SPAN element. I have not
> checked if other variations works: this one does.
>
> What do you think?

The float thing was the second suggestion I was given on www-style.
Can you provide a patch I can apply to my tree for testing to see how
it comes up?


-- 
Giuseppe "Oblomov" Bilotta

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [RFC/PATCH] gitweb: Fix nested links problem with ref markers
  2009-01-13  0:59     ` Giuseppe Bilotta
@ 2009-01-14  0:17       ` Jakub Narebski
  2009-01-14  3:56         ` Giuseppe Bilotta
  0 siblings, 1 reply; 8+ messages in thread
From: Jakub Narebski @ 2009-01-14  0:17 UTC (permalink / raw)
  To: Giuseppe Bilotta; +Cc: git

On Tue, 13 Jan 2009, Giuseppe Bilotta wrote:
> On Mon, Jan 12, 2009 at 7:13 PM, Jakub Narebski <jnareb@gmail.com> wrote:
>> On Mon, 12 Jan 2009, Giuseppe Bilotta wrote:
>>> On Sun, Jan 11, 2009 at 8:15 PM, Jakub Narebski <jnareb@gmail.com> wrote:

[...]
> Notice that nested links are actually valid *XML*. Indeed, I asked on
> www-style and they suggested leaving the problem as-is, serving as
> html+xml which is what we do.

But nested links are neither valid HTML nor valid XHTML.  This
restriction (no nested links) is IMVHO quite sensible, but IIRC
it simply cannot be expressed as pure XML restriction (DTD, XML Schema,
Relax-NG... perhaps Sablotron can express this restriction).

>>>>  * Revert 4afbaef (we lose feature, but how often used is it?)
>>>>  * Always use quirks mode, or check browser and use quirks mode if it
>>>>    would break layout
>>>>  * Use extra divs and links and CSS positioning to make layout which
>>>>    looks like current one, and behaves as current one, but is more
>>>>    complicated.
>>>
>>> I'm asking on #html, hopefully I'll get some interesting idea to try for this.
>>
>> I have found _a_ solution. Perhaps not the best one, but it works.
>> And IMHO gives / can give even better visual.
>>
>> Current version (line wrapped for better visibility):
[...]
>> Proposed code (line wrapped for better visibility, with CSS embedded,
>> which would change in final version of course). Only parts of style
>> related to positioning are shown.
>>  <div class="header">
>>  <a href="..." style="float: left; margin: 6px 1px 6px 8px;">GIT 1.6.1</a>
>>  <div style="float: left; margin: 6px 1px;">
>>    <span class="refs">
>>      <span class="tag indirect" title="tags/v1.6.1">
>>      <a href="...">v1.6.1</a>
>>      </span>
>>    </span>
>>  </div>
>>  <a href="..." style="display: block; padding: 6px 8px;">&nbsp;</a>
>>  </div>

[...]
>> What do you think?
> 
> The float thing was the second suggestion I was given on www-style.

What was the first suggestion? And what is www-style?

> Can you provide a patch I can apply to my tree for testing to see how
> it comes up?

Here it is. Note that CSS could be I think reduced. The size of
gitweb.perl changes is affected by changing calling convention for
git_print_header_html subroutine.

There is also strange artifact at least in Mozilla 1.17.2: if I hover
over ref marker, the subject (title) gets darker background. Curious...

-- >8 --
From: Jakub Narebski <jnareb@gmail.com>
Date: Wed, 14 Jan 2009 01:07:30 +0100
Subject: [RFC/PATCH] gitweb: Fix nested links problem with ref markers

Now that ref markers are links, they create nested links which are not
allowed in HTML, and in strict mode in some browsers it causes layout
errors.

It fixes format_subject_html subroutine (used for example in
'shortlog' view) to put $extra (which currently is formatted ref
marker) outside "subject" link, as $extra can contain links and in
HTML links cannot be nested.

It changes calling convention of git_print_header_div (and accordingly
its calling sites) to provide $ref as separate argument, instead of
concatenating it and putting it in $title, and provides workaround so
links are not nested.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.css  |   25 +++++++++++++++++++++-
 gitweb/gitweb.perl |   56 ++++++++++++++++++++++++++++++++-------------------
 2 files changed, 58 insertions(+), 23 deletions(-)

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index a01eac8..244eea2 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -76,23 +76,44 @@ div.page_body {
 	font-family: monospace;
 }
 
-div.title, a.title {
+div.title, a.block {
 	display: block;
 	padding: 6px 8px;
+	text-decoration: none;
+	background-color: #edece6;
+}
+
+div.title, a.title {
 	font-weight: bold;
 	background-color: #edece6;
 	text-decoration: none;
 	color: #000000;
 }
 
+div.header div.extra,
+div.header a.float {
+	float: left;
+	margin: 6px 1px;
+}
+
+div.header a.float {
+	margin: 6px 1px 6px 8px;
+}
+
 div.readme {
 	padding: 8px;
 }
 
-a.title:hover {
+a.block:hover,
+div.header:hover a.title {
 	background-color: #d9d8d1;
 }
 
+div.header a.title:hover {
+	background-color: #edece6; /* ??? */
+	text-decoration: underline;
+}
+
 div.title_text {
 	padding: 6px 0px;
 	border: solid #d9d8d1;
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 0ac84d1..9c192ba 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1408,6 +1408,7 @@ sub format_ref_marker {
 }
 
 # format, perhaps shortened and with markers, title line
+# $extra can contain links, and nested links are illegal in HTML
 sub format_subject_html {
 	my ($long, $short, $href, $extra) = @_;
 	$extra = '' unless defined($extra);
@@ -1415,10 +1416,10 @@ sub format_subject_html {
 	if (length($short) < length($long)) {
 		return $cgi->a({-href => $href, -class => "list subject",
 		                -title => to_utf8($long)},
-		       esc_html($short) . $extra);
+		       esc_html($short)) . $extra;
 	} else {
 		return $cgi->a({-href => $href, -class => "list subject"},
-		       esc_html($long)  . $extra);
+		       esc_html($long))  . $extra;
 	}
 }
 
@@ -3146,17 +3147,30 @@ sub format_paging_nav {
 ## functions printing or outputting HTML: div
 
 sub git_print_header_div {
-	my ($action, $title, $hash, $hash_base) = @_;
+	my ($action, $title, $ref, $hash, $hash_base) = @_;
 	my %args = ();
 
 	$args{'action'} = $action;
 	$args{'hash'} = $hash if $hash;
 	$args{'hash_base'} = $hash_base if $hash_base;
 
-	print "<div class=\"header\">\n" .
-	      $cgi->a({-href => href(%args), -class => "title"},
-	      $title ? $title : $action) .
-	      "\n</div>\n";
+	my $href = href(%args);
+	if ($ref) {
+		# $ref can contain links, and nested links are illegal in HTML
+		# that is why we do this trick (see also gitweb.css for style)
+		# of title text, ref markers and 'background' link
+		print "<div class=\"header\">\n" .
+		      $cgi->a({-href => $href, -class => "title float"},
+		              $title ? $title : $action) . "\n" .
+		      "<div class=\"extra\">\n$ref\n</div>\n" .
+		      $cgi->a({-href => $href, -class => "block"}, '&nbsp;') .
+		      "\n</div>\n"; # class="header"
+	} else {
+		print "<div class=\"header\">\n" .
+		      $cgi->a({-href => $href, -class => "title block"},
+		              $title ? $title : $action) .
+		      "\n</div>\n"; # class="header"
+	}
 }
 
 #sub git_print_authorship (\%) {
@@ -3781,7 +3795,7 @@ sub git_patchset_body {
 	while ($patch_line) {
 
 		# parse "git diff" header line
-		if ($patch_line =~ m/^diff --git (\"(?:[^\\\"]*(?:\\.[^\\\"]*)*)\"|[^ "]*) (.*)$/) {
+		if ($patch_line =~ m/^diff --git (\"(?:[^\\\"]*(?:\\.[^\\\"]*)*)\"|[^ \"]*) (.*)$/) {
 			# $1 is from_name, which we do not use
 			$to_name = unquote($2);
 			$to_name =~ s!^b/!!;
@@ -4523,7 +4537,7 @@ sub git_tag {
 		die_error(404, "Unknown tag object");
 	}
 
-	git_print_header_div('commit', esc_html($tag{'name'}), $hash);
+	git_print_header_div('commit', esc_html($tag{'name'}), undef, $hash);
 	print "<div class=\"title_text\">\n" .
 	      "<table class=\"object_header\">\n" .
 	      "<tr>\n" .
@@ -4591,7 +4605,7 @@ sub git_blame {
 		$cgi->a({-href => href(action=>"blame", file_name=>$file_name)},
 		        "HEAD");
 	git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
-	git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
+	git_print_header_div('commit', esc_html($co{'title'}), undef, $hash_base);
 	git_print_page_path($file_name, $ftype, $hash_base);
 
 	# page body
@@ -4797,7 +4811,7 @@ sub git_blob {
 				        "raw");
 		}
 		git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
-		git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
+		git_print_header_div('commit', esc_html($co{'title'}), undef, $hash_base);
 	} else {
 		print "<div class=\"page_nav\">\n" .
 		      "<br/><br/></div>\n" .
@@ -4870,7 +4884,7 @@ sub git_tree {
 			push @views_nav, $snapshot_links;
 		}
 		git_print_page_nav('tree','', $hash_base, undef, undef, join(' | ', @views_nav));
-		git_print_header_div('commit', esc_html($co{'title'}) . $ref, $hash_base);
+		git_print_header_div('commit', esc_html($co{'title'}), $ref, $hash_base);
 	} else {
 		undef $hash_base;
 		print "<div class=\"page_nav\">\n";
@@ -5009,7 +5023,7 @@ sub git_log {
 		my %ad = parse_date($co{'author_epoch'});
 		git_print_header_div('commit',
 		               "<span class=\"age\">$co{'age_string'}</span>" .
-		               esc_html($co{'title'}) . $ref,
+		               esc_html($co{'title'}), $ref,
 		               $commit);
 		print "<div class=\"title_text\">\n" .
 		      "<div class=\"log_link\">\n" .
@@ -5097,9 +5111,9 @@ sub git_commit {
 	                   $formats_nav);
 
 	if (defined $co{'parent'}) {
-		git_print_header_div('commitdiff', esc_html($co{'title'}) . $ref, $hash);
+		git_print_header_div('commitdiff', esc_html($co{'title'}), $ref, $hash);
 	} else {
-		git_print_header_div('tree', esc_html($co{'title'}) . $ref, $co{'tree'}, $hash);
+		git_print_header_div('tree', esc_html($co{'title'}), $ref, $co{'tree'}, $hash);
 	}
 	print "<div class=\"title_text\">\n" .
 	      "<table class=\"object_header\">\n";
@@ -5292,7 +5306,7 @@ sub git_blobdiff {
 		git_header_html(undef, $expires);
 		if (defined $hash_base && (my %co = parse_commit($hash_base))) {
 			git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
-			git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
+			git_print_header_div('commit', esc_html($co{'title'}), undef, $hash_base);
 		} else {
 			print "<div class=\"page_nav\"><br/>$formats_nav<br/></div>\n";
 			print "<div class=\"title\">$hash vs $hash_parent</div>\n";
@@ -5462,7 +5476,7 @@ sub git_commitdiff {
 
 		git_header_html(undef, $expires);
 		git_print_page_nav('commitdiff','', $hash,$co{'tree'},$hash, $formats_nav);
-		git_print_header_div('commit', esc_html($co{'title'}) . $ref, $hash);
+		git_print_header_div('commit', esc_html($co{'title'}), $ref, $hash);
 		git_print_authorship(\%co);
 		print "<div class=\"page_body\">\n";
 		if (@{$co{'comment'}} > 1) {
@@ -5579,7 +5593,7 @@ sub git_history {
 
 	git_header_html();
 	git_print_page_nav('history','', $hash_base,$co{'tree'},$hash_base, $paging_nav);
-	git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
+	git_print_header_div('commit', esc_html($co{'title'}), undef, $hash_base);
 	git_print_page_path($file_name, $ftype, $hash_base);
 
 	git_history_body(\@commitlist, 0, 99,
@@ -5660,13 +5674,13 @@ sub git_search {
 		}
 
 		git_print_page_nav('','', $hash,$co{'tree'},$hash, $paging_nav);
-		git_print_header_div('commit', esc_html($co{'title'}), $hash);
+		git_print_header_div('commit', esc_html($co{'title'}), undef, $hash);
 		git_search_grep_body(\@commitlist, 0, 99, $next_link);
 	}
 
 	if ($searchtype eq 'pickaxe') {
 		git_print_page_nav('','', $hash,$co{'tree'},$hash);
-		git_print_header_div('commit', esc_html($co{'title'}), $hash);
+		git_print_header_div('commit', esc_html($co{'title'}), undef, $hash);
 
 		print "<table class=\"pickaxe search\">\n";
 		my $alternate = 1;
@@ -5735,7 +5749,7 @@ sub git_search {
 
 	if ($searchtype eq 'grep') {
 		git_print_page_nav('','', $hash,$co{'tree'},$hash);
-		git_print_header_div('commit', esc_html($co{'title'}), $hash);
+		git_print_header_div('commit', esc_html($co{'title'}), undef, $hash);
 
 		print "<table class=\"grep_search\">\n";
 		my $alternate = 1;
-- 
1.6.0.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [RFC/PATCH] gitweb: Fix nested links problem with ref markers
  2009-01-14  0:17       ` [RFC/PATCH] gitweb: Fix nested links problem with ref markers Jakub Narebski
@ 2009-01-14  3:56         ` Giuseppe Bilotta
  2009-01-14 10:39           ` Jakub Narebski
  0 siblings, 1 reply; 8+ messages in thread
From: Giuseppe Bilotta @ 2009-01-14  3:56 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git

On Tue, Jan 13, 2009 at 7:17 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> On Tue, 13 Jan 2009, Giuseppe Bilotta wrote:
>> On Mon, Jan 12, 2009 at 7:13 PM, Jakub Narebski <jnareb@gmail.com> wrote:
>>> On Mon, 12 Jan 2009, Giuseppe Bilotta wrote:
>>>> On Sun, Jan 11, 2009 at 8:15 PM, Jakub Narebski <jnareb@gmail.com> wrote:
>
> [...]
>> Notice that nested links are actually valid *XML*. Indeed, I asked on
>> www-style and they suggested leaving the problem as-is, serving as
>> html+xml which is what we do.
>
> But nested links are neither valid HTML nor valid XHTML.  This
> restriction (no nested links) is IMVHO quite sensible

Sensible? I think it's idiotic, especially since CSS does not and
cannot provide enough expression power to obtain the same layout and
functionality as what can be achieved by nesting links, so that to
achieve these results one has to resort to horirble trickes,
effectively destroying the whole purporse of HTML+CSS which is to
separate layout from semantics.

But anyway.

>> The float thing was the second suggestion I was given on www-style.
>
> What was the first suggestion? And what is www-style?

The first suggestion was to just leave things as they are. www-style
is www-style@w3c.org, the CSS mailing list of the W3C

>> Can you provide a patch I can apply to my tree for testing to see how
>> it comes up?
>
> Here it is. Note that CSS could be I think reduced. The size of
> gitweb.perl changes is affected by changing calling convention for
> git_print_header_html subroutine.

It's funny, I was working on a very similar patch myself a couple of
days ago, but couldn't get the horizontal filler after the link to
work properly, which is why I asked on www-style.

I'll test your patch and let you know.

> There is also strange artifact at least in Mozilla 1.17.2: if I hover
> over ref marker, the subject (title) gets darker background. Curious...

Might be some kind of bug with the capturing vs bubbling phase.
-- 
Giuseppe "Oblomov" Bilotta

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC/PATCH] gitweb: Fix nested links problem with ref markers
  2009-01-14  3:56         ` Giuseppe Bilotta
@ 2009-01-14 10:39           ` Jakub Narebski
  2009-01-14 13:52             ` Giuseppe Bilotta
  0 siblings, 1 reply; 8+ messages in thread
From: Jakub Narebski @ 2009-01-14 10:39 UTC (permalink / raw)
  To: Giuseppe Bilotta; +Cc: git

On Wed, 14 Jan 2009, Giuseppe Bilotta wrote:
> On Tue, Jan 13, 2009 at 7:17 PM, Jakub Narebski <jnareb@gmail.com> wrote:
>> On Tue, 13 Jan 2009, Giuseppe Bilotta wrote:

[...]
>>> The float thing was the second suggestion I was given on www-style.
>>
>> What was the first suggestion? And what is www-style?
> 
> The first suggestion was to just leave things as they are. www-style
> is www-style@w3c.org, the CSS mailing list of the W3C

Thanks.
 
>>> Can you provide a patch I can apply to my tree for testing to see how
>>> it comes up?
>>
>> Here it is. Note that CSS could be I think reduced. The size of
>> gitweb.perl changes is affected by changing calling convention for
>> git_print_header_html subroutine.
> 
> It's funny, I was working on a very similar patch myself a couple of
> days ago, but couldn't get the horizontal filler after the link to
> work properly, which is why I asked on www-style.
> 
> I'll test your patch and let you know.

I am checking 'log' view of git repository; it should have enough
ref markers to test this issue.

It works also in Konqueror 3.5.3-0.2.fc4...
 
>> There is also strange artifact at least in Mozilla 1.17.2: if I hover
>> over ref marker, the subject (title) gets darker background. Curious...
> 
> Might be some kind of bug with the capturing vs bubbling phase.

...but the same artifact can be seen too.  Also I am not entirely
pleased with the way things behave on mouseover.  It is a pity that
you cannot style element using CSS2.1 based on the pseudo-class :hover
of descendant element, or/and pseudo-class of sibling element, which
nevertheless overlays given element.

-- 
Jakub Narebski
Poland

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC/PATCH] gitweb: Fix nested links problem with ref markers
  2009-01-14 10:39           ` Jakub Narebski
@ 2009-01-14 13:52             ` Giuseppe Bilotta
  0 siblings, 0 replies; 8+ messages in thread
From: Giuseppe Bilotta @ 2009-01-14 13:52 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git

On Wed, Jan 14, 2009 at 5:39 AM, Jakub Narebski <jnareb@gmail.com> wrote:
> On Wed, 14 Jan 2009, Giuseppe Bilotta wrote:
>> On Tue, Jan 13, 2009 at 7:17 PM, Jakub Narebski <jnareb@gmail.com> wrote:
>>> On Tue, 13 Jan 2009, Giuseppe Bilotta wrote:
>
>>>> Can you provide a patch I can apply to my tree for testing to see how
>>>> it comes up?
>>>
>>> Here it is. Note that CSS could be I think reduced. The size of
>>> gitweb.perl changes is affected by changing calling convention for
>>> git_print_header_html subroutine.
>>
>> It's funny, I was working on a very similar patch myself a couple of
>> days ago, but couldn't get the horizontal filler after the link to
>> work properly, which is why I asked on www-style.
>>
>> I'll test your patch and let you know.
>
> I am checking 'log' view of git repository; it should have enough
> ref markers to test this issue.
>
> It works also in Konqueror 3.5.3-0.2.fc4...
>
>>> There is also strange artifact at least in Mozilla 1.17.2: if I hover
>>> over ref marker, the subject (title) gets darker background. Curious...
>>
>> Might be some kind of bug with the capturing vs bubbling phase.
>
> ...but the same artifact can be seen too.  Also I am not entirely
> pleased with the way things behave on mouseover.  It is a pity that
> you cannot style element using CSS2.1 based on the pseudo-class :hover
> of descendant element, or/and pseudo-class of sibling element, which
> nevertheless overlays given element.

I know, I've been needing something like this in other occasion. And
that's precisely what I was talking about for the limits of CSS, and
why I really wonder if the illegal XHMTL but valid XML shouldn't
rather be our option ...



-- 
Giuseppe "Oblomov" Bilotta

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2009-01-14 13:53 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-12  1:15 [BUG/RFH] gitweb: Trouble with ref markers being hyperlinks because of illegally nested links Jakub Narebski
2009-01-12  2:59 ` Giuseppe Bilotta
2009-01-13  0:13   ` Jakub Narebski
2009-01-13  0:59     ` Giuseppe Bilotta
2009-01-14  0:17       ` [RFC/PATCH] gitweb: Fix nested links problem with ref markers Jakub Narebski
2009-01-14  3:56         ` Giuseppe Bilotta
2009-01-14 10:39           ` Jakub Narebski
2009-01-14 13:52             ` Giuseppe Bilotta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).