[PATCH] gitweb.js: Harden setting blamed commit info in incremental blame

[Jakub Narebski <jnareb@gmail.com>]

On Sat, 21 Nov 2009, Jakub Narebski wrote:
> On Sat, 21 Nov 2009, Jakub Narebski wrote:
> 
> > * Testing it with IE8 (Internet Explorer 8.0.6001.18702) page loading stops
> >   at 0%, at the very beginning on startBlame() function
> > 
> >   IE8 shows that it finds the following errors:
> > 
> >   * "firstChild is null or not an object"
> >     line: 565, char:4
> > 
> >       a_sha1.firstChild.data = commit.sha1.substr(0, 8);
> > 
> >     It might be caused by the fact that firstChild for this case should be
> >     text node containing of pure whitespace:
> >        <a href=""> </a>
> >     Perhaps IE8 simplifies it in "compatibility view" mode
> 
> This bug (be it in gitweb.js or in IE8) is fixed by the following patch:
> 
> -- 8< --
> diff --git i/gitweb/gitweb.js w/gitweb/gitweb.js
> index 200ec5a..c1e425c 100644
> --- i/gitweb/gitweb.js
> +++ w/gitweb/gitweb.js
> @@ -562,7 +562,12 @@ function handleLine(commit, group) {
>  			td_sha1.rowSpan = group.numlines;
>  
>  			a_sha1.href = projectUrl + 'a=commit;h=' + commit.sha1;
> -			a_sha1.firstChild.data = commit.sha1.substr(0, 8);
> +			if (a_sha1.firstChild) {
> +				a_sha1.firstChild.data = commit.sha1.substr(0, 8);
> +			} else {
> +				a_sha1.appendChild(
> +					document.createTextNode(commit.sha1.substr(0, 8)));
> +			}
>  			if (group.numlines >= 2) {
>  				var fragment = document.createDocumentFragment();
>  				var br   = document.createElement("br");
> -- >8 --

Below the same patch is in the form of a proper commit; although the title
(subject) of this commit could be better...

> >  * "Unspecified error" (twice)
> >    line: 777, char:2
> > 
> >      if (xhr.readyState === 3 && xhr.status !== 200) {
> >      	return;
> >      }
> > 
> >    I don't know what might be the source of error here; I suspect that the
> >    error position mentioned by IE8 is bogus.
> 
> But I have no idea how to fix this.  "Unspecified error" isn't very 
> helpful...

Debugging this is serious PITA.  After fix below it appears that this bug
is some intermittent bug, depending on XMLHttpRequest timing.  It more
often than not (at least when I tried to debug it using build-in IE8
debugger) works correctly for the folowing files: README, GIT-VERSION-GEN,
revision.c (once even it did fail when first running for given file, and
then running correctly when reloading from debugger; fun it is not).

It does consistently fail for gitweb/gitweb.perl... but when I tried
to debug it IE8 would hang up when trying to use debugger (with around
600MB available RAM).  Perhaps somebody else would have more luck...

-- >8 --
Subject: [PATCH] gitweb.js: Harden setting blamed commit info in incremental blame

Internet Explorer 8 stops at beginning of blame filling
with the following bug
  "firstChild is null or not an object"
at this line
  a_sha1.firstChild.data = commit.sha1.substr(0, 8);

It is (probably) caused by the fact that while a_sha1 element,
which looks like this
  <a href=""> </a>
has firstChild which is text node containing only whitespace (single
space character) in other web browsers (Firefox 3.5, Opera 10,
Google Chrome 3.0), IE8 simplifies DOM, removing trailing/leading
whitespace.

Protect against this bug by creating text element if it does not
exist.

Found-by: Stephen Boyd <bebarino@gmail.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.js |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.js b/gitweb/gitweb.js
index 200ec5a..c1e425c 100644
--- a/gitweb/gitweb.js
+++ b/gitweb/gitweb.js
@@ -562,7 +562,12 @@ function handleLine(commit, group) {
 			td_sha1.rowSpan = group.numlines;
 
 			a_sha1.href = projectUrl + 'a=commit;h=' + commit.sha1;
-			a_sha1.firstChild.data = commit.sha1.substr(0, 8);
+			if (a_sha1.firstChild) {
+				a_sha1.firstChild.data = commit.sha1.substr(0, 8);
+			} else {
+				a_sha1.appendChild(
+					document.createTextNode(commit.sha1.substr(0, 8)));
+			}
 			if (group.numlines >= 2) {
 				var fragment = document.createDocumentFragment();
 				var br   = document.createElement("br");
-- 
1.6.5.3