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