Fixing a Bug In A Harmless Test Environment

This week's DPS909 lab brought together everything covered thus far in the course: build the software, file a bug, find the offending code, make a patch. If this were any other course this lab might qualify as a "quest": not a quiz, not a test, but somewhere delightfully in between!

Step one was compiling a debug build of Thunderbird/Shredder. I checked out straight from the repo rather than use the code provided by the lab instructions. The process was similar to building Firefox/Minefield and went off literally without a hitch. A very palpable hit for me!

Nest was to confirm the bug's existence. The bug we were interested in deals with Thunderbird converting a string that looks like an email address into a hyperlink; the problem is the string does not represent a valid email address and so should not be converted. We can see the fubared behaviour below:


So having confirmed the existence of the bug without difficulty, bug #7243 was filed in the landfill. I like to think I included the bare-bones information in the description answering the questions of: "what is wrong?", "how may it be reproduced?", and "what version of the software are you using?".

Now came the matter of tracking down that needing futzing with. This took a while. I spent much time plopping in search terms into the Mozilla Cross-Reference without success. I looked into "parse message", "save draft", and the like to try and get into where the message was stored and perhaps displayed to the screen.

In desperation I went back to the program and, mousing over the offending email link, saw it was a mailto link. So I through in mailto as the search term and... there was light. (Just goes to show simplicity is golden.) Conveniently the very first result from that search took me exactly where I needed to go.

if (inString.FindChar('.', pos) != kNotFound)

The conditional only checked if a "." was found after the @ sign. What it also needed to do was check if there were any ".." after the @ too, which would mean it was not a valid address. Some way needed to be found to find whether a ".." was in the string. Unlike what was currently in mozTXTToHTMLConv.cpp I needed a way to check the presence of a string in the string, and not just a char in the string. According to the Mozilla internal string guide the correct way to find if a substring existed was to useFindInReadable in nsScannerString.cpp, However converting the code to use the newer approved way was probably a bit beyond what I wanted to delve into for this lab.

So I just stuck with the old way. Searching through the documentation some more I found nsDependentString had just the function I needed. The code mozTXTToHTMLConv changed to:

if (inString.FindChar('.', pos) != kNotFound && inString.Find("..", pos) == kNotFound)

A recompile later the situation was much improved:


I created the patch from ./comm-central/mozilla and ran it through JST Review. A few "longer than 80 characters" and extra whitespace/tabs warnings later, it was ready to be attached to the bug report.

Ehren graciously took some time to review the patch and found I had indented a line a bit too far. It was up to a patched patch to pass muster instead.

The tool-tips for the review options for attachments threw me off a bit. Super-reviews are only needed for world-shaking changes, but the super-review tool-tip says something somewhat different. I didn't request a super-review for the revised patch.

The patch and review comments have been copied to my Seneca College wiki page.