Here are a few tips for writing better code and things that you may be asked to do during a review:
Anything non-obvious in code should be documented with a comment. This includes function parameters, return values, invalid conditions, assumptions etc.
#include toolkit/content/debug.js
(already present in the
browser window) and use NS_ASSERT() to make your assumptions explicit.
When an assertion fails in chrome, a dialog will be shown with the
message you supply and a call stack to that point, which can be useful
for debugging purposes, especially as nightly testers report this in
bugs.
It goes without saying, use good, descriptive yet succinct names for variables, functions, properties, etc.
For new code, don't prefix parameters with 'a', or member variables with 'm'. Use '_' to prefix member variables instead.
Avoid globals at all costs. Where possible, create an object with a descriptive name for your set of functions and make them methods and properties on it. This ensures features are all compartmentalized from each other and avoid collisions. Where you must have a global, prefix with 'g'.
All class names and wrapper objects should start with an uppercase letter.
All names should use interCaps not under_scores. This goes for file names too.
When editing existing code, if the file is consistent in its style, conform to it even if that is different to something you see elsewhere. Maintaining neatness and consistency is important for readability.
When you edit files that are a mess - like browser.js, don't just get in and get out as quickly as possible, as tempting as it can be. Think about what you're doing and choose the best solution, even if it seems everyone else isn't. That's the only way we can make meta-improvements. When you figure out something complicated, leave a comment explaining what it does.
For large self contained subsections in XUL files, consider using the preprocessor to include them from separate files. This makes the overall structure of the file easier to understand at a glance.
Avoid using the preprocessor to include JavaScript because it messes with line numbers in error reporting, making debugging difficult.
The browser window, its command handling infrastructure and small
dialogs relating to it go in browser/base/
Major browser features are in browser/components/
.
Code should not go into the toolkit/ directory unless it needs to be
shared explicitly by Thunderbird (in which case it should go into the
toolkit/mozapps/
directory) or it is a basic requirement
of the application toolkit for all applications like Songbird, etc.
In general, there should be a demand for your addition and a commitment
to use it. Adding code to toolkit can mean it implicitly becomes part of
the XUL API. XUL isn't frozen, but we don't like to change it without
good reason.