Posts Tagged JavaScript
v0.7 Release - Reducing Code Duplication
Posted by Sid in DPS911, Mercurial Project, Open Source on March 14th, 2009
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.
v0.7Release - Retrieving the Total Number of Entries
Posted by Sid in DPS911, Mercurial Project, Open Source on March 12th, 2009
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.
v0.6 Release - Fixing Annotate for the Paper Theme
Posted by Sid in DPS911, Mercurial Project, Open Source on February 17th, 2009
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.
v0.5 Release Goals
Posted by Sid in DPS911, Mercurial Project, Open Source on January 28th, 2009
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.
v0.4 Release Complete
Posted by Sid in DPS911, Mercurial Project, Open Source on January 23rd, 2009
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:
- The release goals
- Optimizing Code
- Better regexps for identifying bugLinks
- Linkifying all bug strings on a line
- Fixing XSS vulnerabilities and loader.gif
- Fixing the unresolved script error for entries loaded onPageLoad
- Fixing the problem with merge changesets background color
View the project page for more details.
v0.4 Release - Loading More Entries onPageLoad
Posted by Sid in DPS911, Mercurial Project, Open Source on January 22nd, 2009
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 < i; counter++) { if(start > 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 > 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.
v0.4 Release - Linkifying all Bug Strings
Posted by Sid in DPS911, Mercurial Project, Open Source on January 18th, 2009
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:
