{"thread":{"id":"17104","subject":"[BUG/RFH] gitweb: Trouble with ref markers being hyperlinks because of illegally nested links","startedAt":"2009-01-12T01:15:11Z","lastAt":"2009-01-14T13:52:03Z","messageCount":8,"participants":["Jakub Narebski","Giuseppe Bilotta"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"100043","messageId":"200901120215.13668.jnareb@gmail.com","threadId":"17104","inReplyTo":null,"subject":"[BUG/RFH] gitweb: Trouble with ref markers being hyperlinks because of illegally nested links","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-01-12T01:15:11Z","receivedAt":"2009-01-12T01:15:11Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Commit 4afbaef by Giuseppe Bilotta (gitweb: ref markers link to named \nshortlogs) turned ref markers for tags and heads into links to \nappropriate views for the ref name.\n\nUnfortunately the code didn't take into account the fact that nesting \nlinks (A elements) is illegal in (X)HTML:\n\n 12.2.2 Nested links are illegal\n\n Links and anchors defined by the A element must not be nested;\n an A element must not contain any other A elements.\n\n(from http://www.w3.org/TR/html401/struct/links.html#h-12.2.2), and that \nsome browsers (e.g. Mozilla 1.17.2 I still use) in the very strict \nconformance mode (application/xhtml+xml mimetype and XML + XHTML DTD) \n_enforce_ this requirement by moving inner link immediately outside the \nend of outer link, i.e. for the HTML source looking like the following\n some text v1.5.1\nrendered HTML (which you can see using \"View Selection Source\") is \ninstead\n some text v1.5.1\nAnd of course SPAN elements which wraps inner link (inner A element) is \n_not_ moved.\n\n\nThis is quite easy to fix for hyperlinked ref markers in 'shortlog' and \n'history' views: just close the \"title\" hyperlink before printing \n$extra, i.e. ref markers. I have even made a patch doing that. Then \ninstead of incorrect\n _Merge branch into maint_ [] _maint_\nwhere _aaa_ means that 'aaa' is hyperlink, and [xxx] is a fer marker,\nwe will have correct:\n _Merge branch into maint_ [_maint_]\nSee that we have two separate and not nested links...\n\n\nWhat is more complicated is the issue of ref marker from \ngit_print_header_div e.g. in 'commit'/'commitdiff' view, and in 'log' \nview. There link is made into block element using \"display: block;\"\nCSS rule (div.title, a.title), so that you can click _anywhere_ on the \nheader block. This breaks layout even worse, making hyperlinked ref \nmarker text appear *below* header div:\n\n -----------------------------------------------------------\n |_Merge branch into maint_ [] |\n -----------------------------------------------------------\n _maint_\n\nTo preserve current layout and behavior it would be needed to do some \ndeep HTML + CSS positioning hackery, perhaps with additional link block \nwithout any text... But I don't know exactly how to do this; all [few] \nexperiments I did failed.\n\nI see possible the following alternate solutions:\n * Ignore this issue (e.g. if it does not affect modern browsers)\n * Revert 4afbaef (we lose feature, but how often used is it?)\n * Always use quirks mode, or check browser and use quirks mode if it\n would break layout\n * Use extra divs and links and CSS positioning to make layout which\n looks like current one, and behaves as current one, but is more\n complicated.\n\n\nP.S. From what I have checked neither kernel.org nor repo.or.cz\nhave this issue.\n-- \nJakub Narebski\nPoland\n"},{"id":"100061","messageId":"cb7bb73a0901111859q3a166d92k5176b27af2c4d256@mail.gmail.com","threadId":"17104","inReplyTo":"200901120215.13668.jnareb@gmail.com","subject":"Re: [BUG/RFH] gitweb: Trouble with ref markers being hyperlinks because of illegally nested links","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-01-12T02:59:58Z","receivedAt":"2009-01-12T02:59:58Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Sun, Jan 11, 2009 at 8:15 PM, Jakub Narebski wrote:\n> Commit 4afbaef by Giuseppe Bilotta (gitweb: ref markers link to named\n> shortlogs) turned ref markers for tags and heads into links to\n> appropriate views for the ref name.\n>\n> Unfortunately the code didn't take into account the fact that nesting\n> links (A elements) is illegal in (X)HTML:\n>\n> 12.2.2 Nested links are illegal\n>\n> Links and anchors defined by the A element must not be nested;\n> an A element must not contain any other A elements.\n>\n> (from http://www.w3.org/TR/html401/struct/links.html#h-12.2.2), and that\n> some browsers (e.g. Mozilla 1.17.2 I still use) in the very strict\n> conformance mode (application/xhtml+xml mimetype and XML + XHTML DTD)\n> _enforce_ this requirement by moving inner link immediately outside the\n> end of outer link, i.e. for the HTML source looking like the following\n> some text v1.5.1\n> rendered HTML (which you can see using \"View Selection Source\") is\n> instead\n> some text v1.5.1\n> And of course SPAN elements which wraps inner link (inner A element) is\n> _not_ moved.\n>\n>\n> This is quite easy to fix for hyperlinked ref markers in 'shortlog' and\n> 'history' views: just close the \"title\" hyperlink before printing\n> $extra, i.e. ref markers. I have even made a patch doing that. Then\n> instead of incorrect\n> _Merge branch into maint_ [] _maint_\n> where _aaa_ means that 'aaa' is hyperlink, and [xxx] is a fer marker,\n> we will have correct:\n> _Merge branch into maint_ [_maint_]\n> See that we have two separate and not nested links...\n\nI've seen from the list that you already have patches ready to fix at\nleast this problem. I think we might start from having these patches\nin while we think of the best way to fix the biggest issue:\n\n> What is more complicated is the issue of ref marker from\n> git_print_header_div e.g. in 'commit'/'commitdiff' view, and in 'log'\n> view. There link is made into block element using \"display: block;\"\n> CSS rule (div.title, a.title), so that you can click _anywhere_ on the\n> header block. This breaks layout even worse, making hyperlinked ref\n> marker text appear *below* header div:\n>\n> -----------------------------------------------------------\n> |_Merge branch into maint_ [] |\n> -----------------------------------------------------------\n> _maint_\n>\n> To preserve current layout and behavior it would be needed to do some\n> deep HTML + CSS positioning hackery, perhaps with additional link block\n> without any text... But I don't know exactly how to do this; all [few]\n> experiments I did failed.\n>\n> I see possible the following alternate solutions:\n> * Ignore this issue (e.g. if it does not affect modern browsers)\n\nThat would be my current choice until we find a better solution.\n\n> * Revert 4afbaef (we lose feature, but how often used is it?)\n> * Always use quirks mode, or check browser and use quirks mode if it\n> would break layout\n> * Use extra divs and links and CSS positioning to make layout which\n> looks like current one, and behaves as current one, but is more\n> complicated.\n\nI'm asking on #html, hopefully I'll get some interesting idea to try for this.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"100201","messageId":"200901130113.54517.jnareb@gmail.com","threadId":"17104","inReplyTo":"cb7bb73a0901111859q3a166d92k5176b27af2c4d256@mail.gmail.com","subject":"Re: [BUG/RFH] gitweb: Trouble with ref markers being hyperlinks because of illegally nested links","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-01-13T00:13:54Z","receivedAt":"2009-01-13T00:13:54Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Mon, 12 Jan 2009, Giuseppe Bilotta wrote:\n> On Sun, Jan 11, 2009 at 8:15 PM, Jakub Narebski wrote:\n\n> > Commit 4afbaef by Giuseppe Bilotta (gitweb: ref markers link to named\n> > shortlogs) turned ref markers for tags and heads into links to\n> > appropriate views for the ref name.\n> >\n> > Unfortunately the code didn't take into account the fact that nesting\n> > links (A elements) is illegal in (X)HTML:\n> >\n> > 12.2.2 Nested links are illegal\n> >\n> > Links and anchors defined by the A element must not be nested;\n> > an A element must not contain any other A elements.\n[...]\n\n> > What is more complicated is the issue of ref marker from\n> > git_print_header_div e.g. in 'commit'/'commitdiff' view, and in 'log'\n> > view. There link is made into block element using \"display: block;\"\n> > CSS rule (div.title, a.title), so that you can click _anywhere_ on the\n> > header block. This breaks layout even worse, making hyperlinked ref\n> > marker text appear *below* header div:\n> >\n> > -----------------------------------------------------------\n> > |_Merge branch into maint_ [] |\n> > -----------------------------------------------------------\n> > _maint_\n> >\n> > To preserve current layout and behavior it would be needed to do some\n> > deep HTML + CSS positioning hackery, perhaps with additional link block\n> > without any text... But I don't know exactly how to do this; all [few]\n> > experiments I did failed.\n> >\n> > I see possible the following alternate solutions:\n> > * Ignore this issue (e.g. if it does not affect modern browsers)\n> \n> That would be my current choice until we find a better solution.\n\nBy the way, how common this error is? Could you check if _your_ web\nbrowser (Firefox, Internet Explorer, Opera, Konqueror, Safari, Chrome)\ndoes show this bug or not, please?\n\n> > * Revert 4afbaef (we lose feature, but how often used is it?)\n> > * Always use quirks mode, or check browser and use quirks mode if it\n> > would break layout\n> > * Use extra divs and links and CSS positioning to make layout which\n> > looks like current one, and behaves as current one, but is more\n> > complicated.\n> \n> I'm asking on #html, hopefully I'll get some interesting idea to try for this.\n\nI have found _a_ solution. Perhaps not the best one, but it works.\nAnd IMHO gives / can give even better visual.\n\nCurrent version (line wrapped for better visibility):\n \n\nCurrent CSS (relevant part):\n a.title {\n \tdisplay: block;\n \tpadding: 6px 8px;\n }\n\nCurrent rendering:\n -----------------------------------------------------------\n |_GIT 1.6.1_ [] |\n -----------------------------------------------------------\n __v1.6.1__\n\n\nProposed code (line wrapped for better visibility, with CSS embedded,\nwhich would change in final version of course). Only parts of style\nrelated to positioning are shown.\n \n\nRendering with proposed code:\n -----------------------------------------------------------\n _|_GIT 1.6.1_ [_v1.6.1_] |_\n -----------------------------------------------------------\n\nI guess that instead of additional DIV element, we could use DIV for\n.refs, or use \"float: right\" style with SPAN element. I have not\nchecked if other variations works: this one does.\n\nWhat do you think?\n-- \nJakub Narebski\nPoland\n"},{"id":"100209","messageId":"cb7bb73a0901121659h7277c074ia73786b15d66b804@mail.gmail.com","threadId":"17104","inReplyTo":"200901130113.54517.jnareb@gmail.com","subject":"Re: [BUG/RFH] gitweb: Trouble with ref markers being hyperlinks because of illegally nested links","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-01-13T00:59:24Z","receivedAt":"2009-01-13T00:59:24Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Mon, Jan 12, 2009 at 7:13 PM, Jakub Narebski wrote:\n> On Mon, 12 Jan 2009, Giuseppe Bilotta wrote:\n>> On Sun, Jan 11, 2009 at 8:15 PM, Jakub Narebski wrote:\n>\n>> > I see possible the following alternate solutions:\n>> > * Ignore this issue (e.g. if it does not affect modern browsers)\n>>\n>> That would be my current choice until we find a better solution.\n>\n> By the way, how common this error is? Could you check if _your_ web\n> browser (Firefox, Internet Explorer, Opera, Konqueror, Safari, Chrome)\n> does show this bug or not, please?\n\nOpera works fine (no display or functionality anomaly). That makes\nsense, since I was the one who submitted the patch 8-D. Konqueror\n3.5.9 does the ugly thing instead.\n\nNotice that nested links are actually valid *XML*. Indeed, I asked on\nwww-style and they suggested leaving the problem as-is, serving as\nhtml+xml which is what we do.\n\n>> > * Revert 4afbaef (we lose feature, but how often used is it?)\n>> > * Always use quirks mode, or check browser and use quirks mode if it\n>> > would break layout\n>> > * Use extra divs and links and CSS positioning to make layout which\n>> > looks like current one, and behaves as current one, but is more\n>> > complicated.\n>>\n>> I'm asking on #html, hopefully I'll get some interesting idea to try for this.\n>\n> I have found _a_ solution. Perhaps not the best one, but it works.\n> And IMHO gives / can give even better visual.\n>\n> Current version (line wrapped for better visibility):\n> \n>\n> Current CSS (relevant part):\n> a.title {\n> display: block;\n> padding: 6px 8px;\n> }\n>\n> Current rendering:\n> -----------------------------------------------------------\n> |_GIT 1.6.1_ [] |\n> -----------------------------------------------------------\n> __v1.6.1__\n>\n>\n> Proposed code (line wrapped for better visibility, with CSS embedded,\n> which would change in final version of course). Only parts of style\n> related to positioning are shown.\n> \n>\n> Rendering with proposed code:\n> -----------------------------------------------------------\n> _|_GIT 1.6.1_ [_v1.6.1_] |_\n> -----------------------------------------------------------\n>\n> I guess that instead of additional DIV element, we could use DIV for\n> .refs, or use \"float: right\" style with SPAN element. I have not\n> checked if other variations works: this one does.\n>\n> What do you think?\n\nThe float thing was the second suggestion I was given on www-style.\nCan you provide a patch I can apply to my tree for testing to see how\nit comes up?\n\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"100344","messageId":"200901140117.38803.jnareb@gmail.com","threadId":"17104","inReplyTo":"cb7bb73a0901121659h7277c074ia73786b15d66b804@mail.gmail.com","subject":"[RFC/PATCH] gitweb: Fix nested links problem with ref markers","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-01-14T00:17:37Z","receivedAt":"2009-01-14T00:17:37Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Tue, 13 Jan 2009, Giuseppe Bilotta wrote:\n> On Mon, Jan 12, 2009 at 7:13 PM, Jakub Narebski wrote:\n>> On Mon, 12 Jan 2009, Giuseppe Bilotta wrote:\n>>> On Sun, Jan 11, 2009 at 8:15 PM, Jakub Narebski wrote:\n\n[...]\n> Notice that nested links are actually valid *XML*. Indeed, I asked on\n> www-style and they suggested leaving the problem as-is, serving as\n> html+xml which is what we do.\n\nBut nested links are neither valid HTML nor valid XHTML. This\nrestriction (no nested links) is IMVHO quite sensible, but IIRC\nit simply cannot be expressed as pure XML restriction (DTD, XML Schema,\nRelax-NG... perhaps Sablotron can express this restriction).\n\n>>>> * Revert 4afbaef (we lose feature, but how often used is it?)\n>>>> * Always use quirks mode, or check browser and use quirks mode if it\n>>>> would break layout\n>>>> * Use extra divs and links and CSS positioning to make layout which\n>>>> looks like current one, and behaves as current one, but is more\n>>>> complicated.\n>>>\n>>> I'm asking on #html, hopefully I'll get some interesting idea to try for this.\n>>\n>> I have found _a_ solution. Perhaps not the best one, but it works.\n>> And IMHO gives / can give even better visual.\n>>\n>> Current version (line wrapped for better visibility):\n[...]\n>> Proposed code (line wrapped for better visibility, with CSS embedded,\n>> which would change in final version of course). Only parts of style\n>> related to positioning are shown.\n>> \n\n[...]\n>> What do you think?\n> \n> The float thing was the second suggestion I was given on www-style.\n\nWhat was the first suggestion? And what is www-style?\n\n> Can you provide a patch I can apply to my tree for testing to see how\n> it comes up?\n\nHere it is. Note that CSS could be I think reduced. The size of\ngitweb.perl changes is affected by changing calling convention for\ngit_print_header_html subroutine.\n\nThere is also strange artifact at least in Mozilla 1.17.2: if I hover\nover ref marker, the subject (title) gets darker background. Curious...\n\n-- >8 --\nFrom: Jakub Narebski \nDate: Wed, 14 Jan 2009 01:07:30 +0100\nSubject: [RFC/PATCH] gitweb: Fix nested links problem with ref markers\n\nNow that ref markers are links, they create nested links which are not\nallowed in HTML, and in strict mode in some browsers it causes layout\nerrors.\n\nIt fixes format_subject_html subroutine (used for example in\n'shortlog' view) to put $extra (which currently is formatted ref\nmarker) outside \"subject\" link, as $extra can contain links and in\nHTML links cannot be nested.\n\nIt changes calling convention of git_print_header_div (and accordingly\nits calling sites) to provide $ref as separate argument, instead of\nconcatenating it and putting it in $title, and provides workaround so\nlinks are not nested.\n\nSigned-off-by: Jakub Narebski \n---\n gitweb/gitweb.css | 25 +++++++++++++++++++++-\n gitweb/gitweb.perl | 56 ++++++++++++++++++++++++++++++++-------------------\n 2 files changed, 58 insertions(+), 23 deletions(-)\n\ndiff --git a/gitweb/gitweb.css b/gitweb/gitweb.css\nindex a01eac8..244eea2 100644\n--- a/gitweb/gitweb.css\n+++ b/gitweb/gitweb.css\n@@ -76,23 +76,44 @@ div.page_body {\n \tfont-family: monospace;\n }\n \n-div.title, a.title {\n+div.title, a.block {\n \tdisplay: block;\n \tpadding: 6px 8px;\n+\ttext-decoration: none;\n+\tbackground-color: #edece6;\n+}\n+\n+div.title, a.title {\n \tfont-weight: bold;\n \tbackground-color: #edece6;\n \ttext-decoration: none;\n \tcolor: #000000;\n }\n \n+div.header div.extra,\n+div.header a.float {\n+\tfloat: left;\n+\tmargin: 6px 1px;\n+}\n+\n+div.header a.float {\n+\tmargin: 6px 1px 6px 8px;\n+}\n+\n div.readme {\n \tpadding: 8px;\n }\n \n-a.title:hover {\n+a.block:hover,\n+div.header:hover a.title {\n \tbackground-color: #d9d8d1;\n }\n \n+div.header a.title:hover {\n+\tbackground-color: #edece6; /* ??? */\n+\ttext-decoration: underline;\n+}\n+\n div.title_text {\n \tpadding: 6px 0px;\n \tborder: solid #d9d8d1;\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 0ac84d1..9c192ba 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1408,6 +1408,7 @@ sub format_ref_marker {\n }\n \n # format, perhaps shortened and with markers, title line\n+# $extra can contain links, and nested links are illegal in HTML\n sub format_subject_html {\n \tmy ($long, $short, $href, $extra) = @_;\n \t$extra = '' unless defined($extra);\n@@ -1415,10 +1416,10 @@ sub format_subject_html {\n \tif (length($short) < length($long)) {\n \t\treturn $cgi->a({-href => $href, -class => \"list subject\",\n \t\t -title => to_utf8($long)},\n-\t\t esc_html($short) . $extra);\n+\t\t esc_html($short)) . $extra;\n \t} else {\n \t\treturn $cgi->a({-href => $href, -class => \"list subject\"},\n-\t\t esc_html($long) . $extra);\n+\t\t esc_html($long)) . $extra;\n \t}\n }\n \n@@ -3146,17 +3147,30 @@ sub format_paging_nav {\n ## functions printing or outputting HTML: div\n \n sub git_print_header_div {\n-\tmy ($action, $title, $hash, $hash_base) = @_;\n+\tmy ($action, $title, $ref, $hash, $hash_base) = @_;\n \tmy %args = ();\n \n \t$args{'action'} = $action;\n \t$args{'hash'} = $hash if $hash;\n \t$args{'hash_base'} = $hash_base if $hash_base;\n \n-\tprint \"\\n\";\n+\tmy $href = href(%args);\n+\tif ($ref) {\n+\t\t# $ref can contain links, and nested links are illegal in HTML\n+\t\t# that is why we do this trick (see also gitweb.css for style)\n+\t\t# of title text, ref markers and 'background' link\n+\t\tprint \"\\n\"; # class=\"header\"\n+\t} else {\n+\t\tprint \"\\n\"; # class=\"header\"\n+\t}\n }\n \n #sub git_print_authorship (\\%) {\n@@ -3781,7 +3795,7 @@ sub git_patchset_body {\n \twhile ($patch_line) {\n \n \t\t# parse \"git diff\" header line\n-\t\tif ($patch_line =~ m/^diff --git (\\\"(?:[^\\\\\\\"]*(?:\\\\.[^\\\\\\\"]*)*)\\\"|[^ \"]*) (.*)$/) {\n+\t\tif ($patch_line =~ m/^diff --git (\\\"(?:[^\\\\\\\"]*(?:\\\\.[^\\\\\\\"]*)*)\\\"|[^ \\\"]*) (.*)$/) {\n \t\t\t# $1 is from_name, which we do not use\n \t\t\t$to_name = unquote($2);\n \t\t\t$to_name =~ s!^b/!!;\n@@ -4523,7 +4537,7 @@ sub git_tag {\n \t\tdie_error(404, \"Unknown tag object\");\n \t}\n \n-\tgit_print_header_div('commit', esc_html($tag{'name'}), $hash);\n+\tgit_print_header_div('commit', esc_html($tag{'name'}), undef, $hash);\n \tprint \"\\n\" .\n \t \"