Posts Tagged JavaScript

v0.7 Release - Reducing Code Duplication

I’ve decided to make a small change for this release along with the other changes that I have already outlined. I want to avoid code duplication as much as possible and thus I have decided to put code to create a rev link into a function. Currently the code for creating a rev link looks like the following:

var tdScrollChangeset = document.createElement("td");
tdScrollChangeset.innerHTML += 
  '<a href=\"/rev/' + 
  pushData[i].individualChangeset.substring(0, 12) + 
  '\">' + 
  pushData[i].individualChangeset.substring(0, 12) + 
  '</a>';

After my change the code looks like the following:

var tdScrollChangeset = createRevLink(pushData[i].individualChangeset);
function createRevLink(str) {
  var td = document.createElement("td");
 
  td.innerHTML +=
    '<a href=\"/rev/' +
    str.substring(0, 12) + 
    '\">' + 
    str.substring(0, 12) +
    '</a>'; 
 
  return td;
}

This alteration reduces the number of lines of code but keeps the same functionality. A win win by all accounts.

, ,

2 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

v0.7 Release - The Scroll Bar Problem

As a part of one of my goals for this release I want to find a solution to the pesky scroll bar problem. Let me explain:

The Problem

Basically, by default the pushlog only loads 10 entries, which doesn’t fill out the entire page and thus the scroll bar does not show up. Now, this means that when the user tries to scroll down (to load more entries) nothing happens as the scroll bar event doesn’t fire. You might be wondering why not just load more entries by default? Well, first of all that isn’t an elegant solution, it’s a cop out and second, different users have varying screen sizes and thus this solution is not feasible.

A dynamic solution is needed where, according to the users screen size, a varying amount of entries are loaded automatically to fill out the page so that the scroll bar appears.

Possible Solutions

This problem is quite frustrating, to be honest. I’ve tried various methods to solve this issue but none have worked ideally. So far, I’ve tried to approach this problem from one angle, using the onload attribute of the body tag through which you can pass in a function that is executed when the page loads. In this function, I call my OnScroll function that loads more entries according to the screen size. So basically a loop is required but this reveals a very frustrating issue. Any kind of loop in this function causes the browser to throw an unresponsive script error. Since, I absolutely have to have a loop to implement the desired functionality, this solution is useless.

Next, I tried to get rid of the unresponsive script error by using setTimeout() which delays the execution of a function by x milliseconds. This gets rid of the script error that the browser was throwing but the browser Gods were still unhappy, as Firefox decided to freeze on me for 10-15secs. Once these seconds passed by the browser unfroze but my solution still didn’t work (no entries were loaded). Basically, nothing happened!

So all in all my attempts to come up with an efficient solution have all been futile, so far.

New Idea

I had a talk with Dave Humphrey this week about this scroll bar problem. He came up with an interesting idea. He approached the problem from a different angle. What if you could get the browser to tell you whether the scroll bar is available or not. So all you would do is keep on adding new entries until the scroll bar appeared and then stop.

I hadn’t thought about the problem in the manner that Dave highlighted. I think he might be onto something here. I’ll have to look into how I can do this in JavaScript. I am certain that there is a way to know whether the scroll bar is present or not. This new lead sounds very promising!

Stay tuned! I’ll be blogging about my findings very soon!

, , ,

No Comments

v0.7Release - Retrieving the Total Number of Entries

Earlier today I outlined the goals of my current release. One of the problems I was looking to fix was with the function, getMaxEntries(). The job of this function was to create an xmlHttpRequest() in order to retrieve the maximum number of push entries in the repository. The pushlog displays data in reverse chronological order and thus I need to know the maximum number of entries so that I know which entries to display first to maintain the same order.

For example in my test repository I have 2613 total entries. The first 10 entries are displayed by default so when the user scrolls down to load more data the very first entry shown would be #2603, then 2602 and so on. Previously, to retrieve the maximum number of entries I was using the following function:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
function getMaxEntries() {
  var entries = new XMLHttpRequest();
  var max = 0;
  entries.open('GET', '/json-pushes?startID=0&endID=1', true);
  entries.onreadystatechange = function() {
  if(entries.readyState == 4)  {
    if(entries.status != 404) {
      var entryData = JSON.parse(entries.responseText);
      max = entryData[1].max;
 
      start = max - 10;
    }
  } else 
      return 0;
  } 
  entries.send(null);
}

I didn’t really like this method of using an xmlHttpRequest() just to retrieve one value. I had to call this function onPageLoad() to calculate the maximum number of entries. This had to be done before my OnScroll function was called or various horrible errors would occur.

I’ve noticed something very unique about JavaScript. It doesn’t wait for one function to finish executing before going on to the next function. If your first function is taking too long, JavaScript will move on to your next function. Now, this feature was causing me quite a few headaches. As I said before, getMaxEntries() had to finish executing completely before the OnScroll function was called, otherwise things would go horribly wrong. getMaxEntries() sets the value of start, which is then used by loadEntries() to retrieve data (called OnScroll). Since getMaxEntries() was taking too long to execute, JavaScript was moving on to loadEntries() without setting the value of start, completely wrecking my logic.

Also, getMaxEntries() was causing Firefox to freeze for 10-15secs for reasons unknown to me. This way of calculating the max number of entries wasn’t very elegant and totally unacceptable. I needed to come up with a better solution.

The Solution

hgweb uses a template system called genshi. Basically, the maximum number of entries is calculated via a database query on the server side. In order to pass this server side variable to the client side I had to add the line 4 to pushloghtml() in hgpoller\pushlog-feed.py:

1
2
3
4
5
6
7
8
9
10
    return tmpl('pushlog',
                changenav=changenav(),
                rev=0,
                max=query.totalentries,
                entries=lambda **x: changelist(limit=0,**x),
                latestentry=lambda **x: changelist(limit=1,**x),
                startdate='startdate' in req.form and req.form['startdate'][0] or '1 week ago',
                enddate='enddate' in req.form and req.form['enddate'][0] or 'now',
                querydescription=query.description(),
                archives=web.archivelist("tip"))

Now, this change ^ allowed me access to this variable on the client side. All I had to do was to use this format: {<var_name>} or #<var_name>#. However, one draw back as far as I know is that I can only access this varialble in HTML, not in JavaScript, which is a significant drawback.

So in order to retrieve this data I added an id attribute the following div tag in hg_templates\gitweb_mozilla\pushlog.tmpl:

1
<div id="#max#" class="page_header">

Now, I could set the value of start to the maximum number of entries:

1
start = $("div").attr("id");

Thus, I can easily pass the correct value of start to loadEntries(), which makes an xmlHttpRequest() to retrieve the correctly ordered data when the user scrolls down.

, , , , , ,

No Comments

v0.6 Release - Fixing Annotate for the Paper Theme

In my last release I had put out an hg annotate fix for the gitweb_mozilla theme. Dirkjan Ochtman, a Mercurial Project developer, noticed my release and asked me to come up with a similar fix for the Mercurial Project’s paper theme. I decided to take up the task and see if I could get similar results for the paper theme as I did with gitweb_mozilla.

The paper theme uses a HUGE table to display the results just like gitweb_mozilla. I tested the current version of the theme using a large 10,000 line cpp file. It gave me a loading time of ~30sec, which is ~10sec longer than gitweb_mozilla.

The following is the code to fix annotate:

map

annotateline = '<div class="l#parity#"><div class="codeauthor"><a href="{url}annotate/{node|short}/{file|urlescape}{sessionvars%urlparameter}#{targetline}" title="{node|short}: {desc|escape|firstline}">{author|user}@{rev}</a></div><a class="codeline" href="#{lineid}" id="{lineid}">{linenumber}</a>{line|escape}</div>'

style-paper.css

div.codeauthor { 
    display:inline-block; 
	width:16ch; 
	text-align: right; 
	color:#999999; 
	text-decoration:none;
	margin-right: 25em;
} 
a.codeline { 
    color:#999999; 
	text-decoration:none; 
	margin:0 10px; 
}                                   
div.l0 {
    background-color:#f6f6f0;
}                                                                    
div.l0, div.l1 { 
    display:block; 
}
pre.completecodeline { 
    font-size: 90%;
	line-height:1.4em; 
	font-family: monospace;
    white-space: pre;
}
div.headrev {
    float: left;
	margin-right: 32em;
	margin-left: 8em;
	font-size: 90%;
	font-weight: bolder;
}
div.headline {
	font-weight: bolder;
	font-size: 90%;
}

I changed the code (see above) and tested annotate again to examine the difference in loading time. I got some surprising results. The reduction in file size was not significant at all. The fix for gitweb_mozilla brought down the file size by 25%. However, in this case the reduction in file size wasn’t nearly as significant. Also the reduction in loading time for gitweb_mozilla was ~15sec but for the paper theme the loading time was only reduced by ~12secs. Currently, on my machine the loading time has gone down from ~30secs to ~18secs.

The speed increase isn’t as signficant for the paper theme as it was for gitweb_mozilla. Why is that? I don’t exactly know. Obviously there are other factors coming into play that aren’t allowing a similar speed boost. Nonetheless, there is a noticeble increase in loading times.

, , , ,

No Comments

v0.6 Release - Minor fix for bug 445560

All the way back in November 2008 I had put out a patch to implement expand and collapse functionality (for merge changesets) for the pushlog. I had gotten an r+ review but I just needed to make some minor adjustments to the patch. Time went on and I totally forgot about implementing that minor fix. I was looking through some of my previous work and then I realized that I had forgotten about this.

The change is pretty simple but important. I just have to use proper naming conventions for variable names, which I wasn’t doing before. This might be the type of thing that is easily overlooked, I was guilty of that myself but it is important that we abide by naming conventions. Why? So that the code is readable when other people inevitably come along to change/read it.

In hgpoller/pushlogfeed.py I changed “Id” to “id” as all other variable names are non-capitalized:

 entry = {"author": ctx.user(),
       "desc": ctx.description(),
       "files": web.listfilediffs(tmpl, ctx.files(), n),
       "rev": ctx.rev(),
       "node": hex(n),
       "tags": nodetagsdict(web.repo, n),
       "branches": nodebranchdict(web.repo, ctx),
       "inbranch": nodeinbranch(web.repo, ctx),
       "hidden": "",
       "push": [],
       "mergerollup": [],
       "id": id
       }

In hg_templates/pushlog.tmpl I changed id to pushid to make it more clearer:

var pushid = $(this).attr("class");
pushid = '.' + pushid.substring(11, pushid.length);
$(pushid).nextAll(pushid).toggle();

, , , , ,

No Comments

v0.5 Release Goals

My v0.4 release is over and done with. In that release I mainly focused on bug 459727 but for my v0.5 release I’ll be taking a look at a problem with hg annotate, bug 459823. The problem here is that hg annotate takes FOREVER to load large files such as ten thousand line cpp files found in mozilla-central. People have already spent a bit of time working on this bug so this gives me some insight intto what I’m required to do.

I had a chat with jorendorff and we discussed what should happen when using hg annotate:

  • Show the first screenful in a second or less
  • Don’t lock up the browser while the rest loads
  • Don’t take forever to load the rest of the file, because I often want to search for a snippet of code

This is an interesting task for me as I’ve never really looked at how firefox renders things on the page. In order to make hg annotate load things faster I’ll have to look at what is more efficient. For example would a page with a table of 10,000+ rows load faster or a page with 10,000+ divs? Right now, I don’t know but I’ll have to find out because as jorendorff put it “the current code just fails all day long” and something must be done about that.

, , ,

No Comments

v0.4 Release Complete

As the title suggests my 0.4 release is complete! I’ve managed to fulfill all the goals (and more) that I set out to complete. I’ve posted a new patch for hg_templates containing all my changes. The following are all the related links:

View the project page for more details.

, , ,

No Comments

v0.4 Release - Loading More Entries onPageLoad

One of my goals for this release was to get the loading more entries onPageLoad functionality working properly concerning bug 459727. Currently I was getting an unresponsive script error. I’ve been struggling with this the last few days. I played around with this problem for a while in the hope of finding a viable solution.

The goal is to get more entries to load onPageLoad in order to fill out the entire page so that the scroll bar shows up. This will allow the user to scroll down and load further entries. However, the problem is that different users can have different screen sizes and/or resolutions and thus it is important that the number of entries loaded is dynamic.

To get rid of the unresponsive script error I tried using the following code:

var start;
function setup()
{
    start = 0;
    getMaxEntries();
 
    var docHeight = document.body.clientHeight;
    var desiredHeight = window.innerHeight;
 
    var i = Math.ceil(window.innerHeight / document.body.clientHeight);
 
    for(var counter = 0; counter &lt; i; counter++) {
        if(start &gt; 0) {
	    loadEntries();
       }
    }
}

The above code doesn’t work. Nothing happens onPageLoad, only the initial set of entries are loaded. I think this is because JavaScript doesn’t wait for getMaxEntries() to finish and moves on to the next line of code thus when the if(start > 0) statement is encountered the value of start is still 0 and loadEntries() is never called.

Somehow, I needed JavaScript to let getMaxEntries() execute completely before moving on to the rest of the code. The value of start is set in getMaxEntries() and loadEntries() will never get called unless the value of start is something greater than 0. I needed some sort of wait() or sleep() functionality. The closest thing I could find to wait() or sleep() in JavaScript was setTimeout(), which delays execution of the function for a set amount of time. The following is the code I came up with:

var start;
function setup()
{
    start = 0;
    getMaxEntries();
 
    var docHeight = document.body.clientHeight;
    var desiredHeight = window.innerHeight;
 
    setTimeout("if(start &gt; 0) {loadEntries(Math.ceil(window.innerHeight / document.body.clientHeight)); }", 1000);
}

Now, the above code sets start to 0 and then executes getMaxEntries(). Then JavaScript moves onto executing the next line but setTimeout() delays the execution of loadEntries(). By the time loadEntires() gets executed the new value of start has been set and loadEntries() works properly.

I tested the above code and it works, more entries are loaded onPageLoad and the scroll bar appears. The only draw back being that Firefox hangs for a bit before the entries load. I don’t know if that is acceptable or not. I’ll put up the new patch tomorrow and lets see what ted and/or jorendorff have to say on this issue.

, , , ,

No Comments

v0.4 Release - Linkifying all Bug Strings

This post is again concerning my fixes for bug 459727. One of my goals for this release is to fix the bug link functionality. I’ve made some progress on that front in terms of creating correct bug links for the differnt types of patterns presented. This time I decided to tackle the other issue with bug links.

The problem is that for any given string that contains bug links only the very first bug string is converted into a link. The subsequent ones fail to get converted. Case in point below:

Both the first bug string “bug 437288″ and the second one, “bug 437288″ should be converted into buglinks but only the first one actually gets converted. This is what I’m looking to fix.

Now, this problem had me stumped for a while. I tried solving it using regexp functions in JavaScript but that was a dead end. I tried to think up other ways but none of them seemed to make sense to me.

Yesterday, while staring at my computer and thinking about this issue it just hit me, a perfect example of the light bulb effect. The answer to my problem was RECURSION!!! A concept I had come to dispise during my data structures and alogrithms class. I had never really used it in any of my programs before. This would be the first time.

Lets jump into the code as I explain this further. I have my main work horse function that creates the bug links:

function createBuglink(str) {
  //Create buglink
  var matchFound = new Boolean(false);
  var bugInDesc = 0;
  var pattern = -1;
  var bugLink = "";
 
  re = new RegExp("[bB][uU][gG]\\s\\d\{6\}");
  re2 = new RegExp("b=\\d\{6\}");
  re3 = new RegExp("\\d\{6\}\\W");
 
  bugInDesc = str.search(re);
  if(bugInDesc != -1) {
    matchFound = true;
	pattern = 1;
  } else {
      bugInDesc = str.search(re2);
	  if(bugInDesc != -1) {
	    matchFound = true;
	    pattern = 2;
	  } else {
	      bugInDesc = str.search(re3);
	      if(bugInDesc != -1) {
	        matchFound = true;
		    pattern = 3;
		  }
	  }
  }
 
  if(matchFound == true) {
    if(pattern == 1) {
	  bugLink = bugLinkify(bugInDesc, str, 4, 10);
	} else if(pattern == 2) {
	  bugLink = bugLinkify(bugInDesc, str, 2, 8);
    } else if(pattern == 3) {
	  bugLink = bugLinkify(bugInDesc, str, 0, 6);
    }	
  } else { //No bug provided
      bugLink = str;
    }
 
  return bugLink;
}

Then I have a function that does the actual string manipulation to insert the link code into the string which is called within createBugLink():

function bugLinkify(bugInDesc, str, start, end) {
  var bugLinkName = str.substring(bugInDesc, bugInDesc + end);
  var bugNumber = bugLinkName.substring(start, end);
  var bugLink = 
    str.substring(0, bugInDesc) + 
    '<a href=\"https://bugzilla.mozilla.org/show_bug.cgi?id=' + 
    bugNumber + 
    '\">' + 
    bugLinkName + 
    '</a>' + 
    createBuglink(str.substring(bugInDesc + end, str.length));
 
  return bugLink;
}

Now, in bugLinkify() the genius of recursion gets to take over. I just call the createBuglink() function again on the rest of the string that didn’t get converted into a buglink and keeps gettig called until there are no bug strings left to be converted.

The upside to this method is that a string could contain an unlimited amount of bug strings and they would all get converted into bug links. But the thing I love most about this method is that I didn’t have to add even a SINGLE line of code. That fact makes this implementation awesome. Gaining functionality without having to add more lines of code is always great.

The following are the results:

, , , , ,

1 Comment