Defect #11612
closed
Revision graph sometimes broken due to raphael.js error
0%
Description
In most Git repositories here, the revision graph is sometimes not completely rendered, some (or even most) lines and dots are missing. I tried it in FF 14 and Safari 6 with the same result.
Firefox error console shows this:
Timestamp: 2012-08-09 11:03:52 Error: TypeError: b[0] is undefined Source File: http://naft02.ch.alcatel-lucent.com/redmine/javascripts/raphael.js?1342787109 Line: 7
Since revision_graph.js does not use any of the deprecated API of Raphaël 2.1, I tried updating Raphaël:
cd public/javascripts
mv raphael.js{,.old}
wget -O raphael.js https://raw.github.com/DmitryBaranovskiy/raphael/master/raphael-min.js
This completely fixes the problem.
Related issues
Updated by Daniel Ritz over 12 years ago
Spoke too soon. Found another case where things are not drawn. Turns out the problem is the missing "space" property of some of the commits. I'll check the root case later. Meanwhile the quick fix in revision_graph.js is this:
diff --git a/public/javascripts/revision_graph.js b/public/javascripts/revision_graph.js
index 31aacd8..92fb7ab 100644
--- a/public/javascripts/revision_graph.js
+++ b/public/javascripts/revision_graph.js
@@ -41,6 +41,9 @@ function drawRevisionGraph(holder, commits_hash, graph_space) {
commits.each(function(commit) {
+ if (!commit.hasOwnProperty("space"))
+ commit.space = 0;
+
y = commit_table_rows[max_rdmid - commit.rdmid].getLayout().get('top') - graph_y_offset + CIRCLE_INROW_OFFSET;
x = graph_x_offset + XSTEP / 2 + XSTEP * commit.space;
@@ -55,6 +58,9 @@ function drawRevisionGraph(holder, commits_hash, graph_space) {
parent_commit = commits_by_scmid.get(parent_scmid);
if (parent_commit) {
+ if (!parent_commit.hasOwnProperty("space"))
+ parent_commit.space = 0;
+
parent_y = commit_table_rows[max_rdmid - parent_commit.rdmid].getLayout().get('top') - graph_y_offset + CIRCLE_INROW_OFFSET;
parent_x = graph_x_offset + XSTEP / 2 + XSTEP * parent_commit.space;
Updated by Jean-Philippe Lang over 12 years ago
- Status changed from New to Resolved
- Assignee set to Jean-Philippe Lang
- Target version set to 2.1.0
- Resolution set to Fixed
Patch applied in r10369. Thanks.
Updated by Etienne Massip over 12 years ago
Not sure forcing the space to 0 has no side-effect, the problem might also be in the commit data.
Updated by Jean-Philippe Lang over 12 years ago
The space property is supposed to be numeric so setting it to 0 if it's undefined can't be bad.
Updated by Etienne Massip over 12 years ago
Jean-Philippe Lang wrote:
The space property is supposed to be numeric so setting it to 0 if it's undefined can't be bad.
IIRC it means forcing the position of the commit on the first displayed branch which is not necessarily correct, that's all my concern.
As Daniel says, having this property unset probably hides some deeper cause.
Updated by Jean-Philippe Lang over 12 years ago
As Daniel says, having this property unset probably hides some deeper cause.
Sure, but his patch fixes his problem. Just some kind of workaround until someone actually fixes the root cause.
Updated by John Kubiatowicz about 12 years ago
I am seeing this problem on 1.4 stable branch as well.
Any chance of a back-ported fix?
Updated by Jens Krämer over 11 years ago
We recently had the same problem - graph rendering broke completely for a repository due to JS errors because some commits were missing the space property.
Turns out that on a fresh install, where the repository in question was re-imported, the problem didn't occur, because the problematic commits didnt show up at all. Further investigation showed that those commits were basically abandoned and not connected to any branch.
I think not rendering these commits in the graph at all is much better than putting them randomly on the first branch as the patch above does.
Here's the patch for 1.4:
--- a/public/javascripts/revision_graph.js
+++ b/public/javascripts/revision_graph.js
@@ -41,6 +41,10 @@ function drawRevisionGraph(holder, commits_hash, graph_space) {
commits.each(function(commit) {
+ if (typeof commit.space != 'undefined') {
y = commit_table_rows[max_rdmid - commit.rdmid].getLayout().get('top') - graph_y_offset + CIRCLE_INROW_OFFSET;
x = graph_x_offset + XSTEP / 2 + XSTEP * commit.space;
@@ -95,6 +99,7 @@ function drawRevisionGraph(holder, commits_hash, graph_space) {
}
top.push(revision_dot_overlay);
+ }
});
top.toFront();
Updated by Etienne Massip over 11 years ago
Jens Krämer wrote:
We recently had the same problem - graph rendering broke completely for a repository due to JS errors because some commits were missing the
spaceproperty.
Could you please open a new issue for this?
Updated by Toshi MARUYAMA over 11 years ago
Etienne Massip wrote:
Jens Krämer wrote:
We recently had the same problem - graph rendering broke completely for a repository due to JS errors because some commits were missing the
spaceproperty.Could you please open a new issue for this?
No need.
r10369 is not in version 1.4.x.
Updated by Toshi MARUYAMA over 11 years ago
Version 1.4.x Prototype and version >= 2.1 jQuery are incompatible.
Updated by Etienne Massip over 11 years ago
Toshi MARUYAMA wrote:
No need.
r10369 is not in version 1.4.x.
?
As I stated in #11612#note-6, r10369 is not a good fix.
It seems that Jens found the true reason of #11612 so he should open an issue for trunk to deal with undefined space property correctly, that is probably more like the way Jens suggested rather than erroneously attaching suspicious commits to first branch.
I didn't say the patch is ready to be applied to trunk, I just say we need to have a correct fix for this issue which is closed and fixed with a released version and, as such, can't be reopen.