From: Jakub Narebski Date: Wed, 25 Nov 2009 00:45:15 GMT Subject: [PATCH] gitweb.js: Harden setting blamed commit info in incremental blame Message-ID: <200911250145.16472.jnareb@gmail.com> In-Reply-To: <200911211556.52175.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: > > > > 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 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 Signed-off-by: Jakub Narebski --- 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