Posts Tagged OnLoad

v0.7 Release - Solving the Split Bug

In my previous blog post I outlined the Split bug which was targeting merge changesets. To fix this issue I re-examined what mergeData contains and was able to come up with a solution. Let me explain why this bug is occurring and how I fixed it:

The Why

There are two types of changesets that are shown by the pushlog, normal changesets and merge changsets. This particular bug applies to merge changesets. When passing merge changeset data from the server side to the client side it is stored in a list, called mergeData. What does this list contain?

1. It contains a another list that contains another list, which contains strings (confusing, isn’t it?)

OR

2. It contains strings

Now, I thought that only the first option was possible and thus I didn’t take the second option into account when writing my code. I still don’t understand how sometimes the list is made up of option 1 while other time, option 2 occurs. Regardless, it does happen and I needed to find a solution

The Fix

Let me show you the code right away. The following is the code that fixes the issue:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
//Check whether it is a merge changeset or not
if(pushData[i].MergeData != []) {
  for(var j = 0; j < pushData[i].mergeData.length; j++) {
    if(pushData[i].mergeData[j] != "") {
      var mergeStr = pushData[i].mergeData[j];
      for(var k = 0; k < pushData[i].mergeData[j].length; k++) {
        if(pushData[i].mergeData[j] instanceof Array) {
          var actualMergeStr = mergeStr[k].split('|-|');
          var mergeC = actualMergeStr[0];
          var mergeUser = actualMergeStr[1];
          var mergeDesc = actualMergeStr[2];
        }
        else {
          var actualMergeStr = mergeStr.split('|-|');
          var mergeC = actualMergeStr[0];
          var mergeUser = actualMergeStr[1];
          var mergeDesc = actualMergeStr[2];
 
          k = pushData[i].mergeData[j].length;
        }
 
        if(mergeDesc != pushData[i].desc) {
          var trScrollMerge = document.createElement("tr");
          trScrollMerge.className = trScroll.className;
          var tdScroll_MergeUser = document.createElement("td");
          tdScroll_MergeUser.width = "184px";
 
          Create changset link 
          var tdScroll_MergeC = document.createElement("td");
            tdScroll_MergeC.innerHTML +=
            '<a href=\"/rev/' +
            mergeC.substring(0, 12) + 
            '\">' + 
            mergeC.substring(0, 12) +
            '</a>'; 
 
          //Create bugLink  
          var merge_bugLink = createBuglink(mergeDesc);
 
          var tdScroll_MergeAuthorDesc = document.createElement("td");
          tdScroll_MergeAuthorDesc.innerHTML += '<strong>' + mergeUser + ' &mdash ' + merge_bugLink + '</strong>';
          trScrollMerge.appendChild(tdScroll_MergeUser);
          trScrollMerge.appendChild(tdScroll_MergeC);
          trScrollMerge.appendChild(tdScroll_MergeAuthorDesc);
          document.getElementById("titlePush").appendChild(trScrollMerge);
        }
      }
    }
  }
}

Basically, line 7 is the key here. The instanceof keyword allows me to find out whether the variable I am examining is a list or not. If it is a list, then mergeData contains option one and I perform the appropriate parsing. However, if it isn’t a list, then option 2 has occurred and I parse the data in a different way.

Adding the above functionality completely solved my problem. No more errors with Split() and all the merge changesets are appearing properly.

,

No Comments

v0.7 Release - Explaining the Split Bug

As a part of one of my goals for this release I want to find a solution for a problem I have appropriately named the “split bug”. Let me explain what it is all about:

Sometimes merge changsets are showing up while others times they aren’t. It has me pretty confused. I wasn’t getting this error beforehand. Basically sometimes I’m getting an error with the Split() function which I’m using to get the rev, user and description for each entry. The error only occurs with some merge changsets, which means they don’t get shown.

Basically, the following is the code I’m using to parse merge changesets right now:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
//Check whether it is a merge changeset or not
if(pushData[i].MergeData != []) {
  for(var j = 0; j < pushData[i].mergeData.length; j++) {
    if(pushData[i].mergeData[j] != "") {
      var mergeStr = pushData[i].mergeData[j];
      for(var k = 0; k < pushData[i].mergeData[j].length; k++) {
        var actualMergeStr = mergeStr[k].split('|-|');
        var mergeC = actualMergeStr[0];
        var mergeUser = actualMergeStr[1];
        var mergeDesc = actualMergeStr[2];
 
        if(mergeDesc != pushData[i].desc) {
          var trScrollMerge = document.createElement("tr");
          trScrollMerge.className = trScroll.className;
          var tdScroll_MergeUser = document.createElement("td");
          tdScroll_MergeUser.width = "184px";
 
          //Create changset link 
          var tdScroll_MergeC = document.createElement("td");
            tdScroll_MergeC.innerHTML +=
            '<a href=\"/rev/' +
            mergeC.substring(0, 12) + 
            '\">' + 
            mergeC.substring(0, 12) +
            '</a>'; 
 
          //Create bugLink  
          var merge_bugLink = createBuglink(mergeDesc);
 
          var tdScroll_MergeAuthorDesc = document.createElement("td");
          tdScroll_MergeAuthorDesc.innerHTML += '<strong>' + mergeUser + ' &mdash ' + merge_bugLink + '</strong>';
          trScrollMerge.appendChild(tdScroll_MergeUser);
          trScrollMerge.appendChild(tdScroll_MergeC);
          trScrollMerge.appendChild(tdScroll_MergeAuthorDesc);
          document.getElementById("titlePush").appendChild(trScrollMerge);
        }
      }
    }
  }
}

This is a very surprising development. Why is this bug popping up now? Was this bug always there or did I make some change to cause it to occur? All things I will have to explore. I think I will start by re-exploring what the JSON data contains. In particular, I need to look at what mergeData contains, in order to find a solution to this bug.

,

No Comments

v0.7 Release - The Scroll Bar Solution

As a part of one of my goals for this release I want to find a solution to the pesky scroll bar problem. I explained the problem and my futile attempts to find a solution in my previous blog post.

So, as I mentioned earlier Dave Humphrey had shown me a different way of looking at the problem. Basically, I want to load more entries until the scroll bar appears. Now, my problem is that I don’t know how you would do this in JavaScript. I asked around and did some research to find a solution. The following is what I came up with:

function fillPage() {
  start = ($("div").attr("id")) - 10;
  var vHeight = 0;
  if (document.documentElement) {
    vHeight = document.documentElement.clientHeight;
  } else {
    vHeight = document.body.clientHeight
  }
 
  if (document.body.offsetHeight &lt; vHeight) {
    //calculate how many times to run loadEntries
    for(var count = 0, offSetHeight = document.body.offsetHeight;
        offSetHeight &lt; vHeight;
        count++, offSetHeight += 180);
 
    if(count &gt; 0)
      loadEntries(count);
  }
}

I get the height of the body and then compare it to the offsetHeight. Basically, I keep on loading more entries until the body height and the body offsetHeight are equal. On larger screens the difference between these two properties will be bigger than on smaller screens.

To do this I increment count by 1 and offSetHeight by 180 each time through the loop. Why 180 you ask? Each time loadEntries(1) is called it loads 20 entries which is at least a height of 180px. Once offSetHeight equals vHeight I pass the value of count to loadEntries() and then according to the value of count more entries are loaded. When count is equal to one 20 entries are loaded, when its value is two then 40 entries are loaded and so on.

This solution completely fixes my problem. I’ve tested it on larger screens in the CDOT area and on my 15inch laptop.  The solution works flawlessly. I’m so glad to finally find a fix for this pesky issue. Unfortunately screenshots won’t be able to prove that this functionality works. I would have to take a video of it in action. For now you’ll have to take my word for it or download the patch (when I upload it) and see for yourself.

, ,

4 Comments