[PATCH] gitweb.js: Harden setting blamed commit info in incremental blame
On Sat, 21 Nov 2009, Jakub Narebski wrote:
Show 39 quoted lines
> 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...
Show 12 quoted lines
> > * "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(-)
Show changes to gitweb/gitweb.js +6 −1
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
Stephen Boyd
