Development:Coding

Here are some useful tips for coding a feature or bug fix for Camino and creating a patch to send your changes through review.

Code Style

Link to Mozilla coding style guidelines as well as create some for Camino, so we avoid Bug 308942 comment 8 and 14 and Bug 228840 comments 15, 16, 18, and 19.

• Mozilla Coding Style Guide
• License block for new files

Also link to Hacking Mozilla general intro somewhere in our contributor introduction

All new code should conform to Camino's style guidelines, which is a descendent of the Mozilla style guidelines (per decree by our leader):

if (foo) {
  bar;
  blam;
}
else {
  baz;
  zap;
}

Braces for single statements are optional (and discouraged, but some still include them).

We explicitly have no style rule regarding the placement of *. Aim to be consistent within a file in as much as the files are consistent.

Coding for latest-OS-version features or using private APIs

Each situation is unique, but we have provided some existing examples of various strategies employed to correctly code for these situations.

Most of the code mentioned below was originally written for Camino 1.6 and allowed Camino to run on Mac OS X version 10.3.9 through 10.5, whether or not the particular feature was supported on that OS or declared in the default Camino SDK (10.4u). The learnWord: code dates from Camino 1.5, which had the same runtime requirements but built against the 10.3.9 SDK on PPC. The code for setting proxies dates from Camino 1.0, which ran on Mac OS X 10.2.8 through 10.4 (and built against the 10.2.8 SDK on PPC, 10.4u SDK on Intel).

The spell-checker's learnWord: method is undocumented on Mac OS X versions older than 10.5, but it is the only way to achieve proper "learned spelling" integration with the Mac OS X spelling system. The code supporting Camino’s use of this method is here and here.

On Mac OS X 10.5, Camino also support building a Language sub-menu for switching spell-check languages.

Setting Spotlight metadata on files is still undocumented as of the 10.5 SDK. Our download listener code has a very well-documented and commented example of how to use a method like this safely while running on Mac OS X 10.4 and 10.5.

See also the Quarantine attributes code for an example of working with features available only on Mac OS X 10.5 while remaining compatible with older versions of the OS.

Setting Mac OS X proxy preferences is available only in the 10.4u SDK and later, although the feature is available at least as far back as Mac OS X 10.3.2. PreferenceManager.mm implements this feature safely across all Mac OS X versions Camino supports; see here.

mento also gives a pseudo-code explanation of how to guard and do runtime look-up for older SDKs in bug 397717 comment 21.

Situation

Description and mxr link

License Blocks

New Files

If you add a new file, be sure to give it the appropriate tri-license license block from http://www.mozilla.org/MPL/boilerplate-1.1/; do not copy boilerplate from other files.

For files added in mozilla/camino, the ”The original code” should be “Camino code.” There are a few general exceptions to this:

1. Makefiles are often “just a lowly Makefile”
2. If you are creating a discrete “sub-module,” e.g. the feed handlers, you can provide a more descriptive name for the module of code
3. If you are contributing code you have written yourself for another project, the original code should be the name of the project from which the code was borrowed, e.g. if Wevah were to contribute punycode-handling code from Paparazzi! and this code was implemented in a new file, the original code would be “Paparazzi!”

Unless you are working on Camino in a work-for-hire situation (or similar), the “Initial Developer” should be your name (optionally followed by your email address). In a work-for-hire situation, the original author is typically your employer; check with your employer for the specifics of your situation.

The copyright year should be the year you began working on the code in the file, even if it's not committed until the following year (e.g., you began developing the patch in December 2009 but the patch was not committed until January 2010).

Finally, list yourself (along with your email address) under “Contributor(s):” and add “(Original Author)” following your name. Note that in work-for-hire situations, even if your employer is listed as the Initial Developer, you should list yourself here.

Moving Existing Code to a New File

If, as part of your patch, you refactor code and move a collection of related code into a new file, you should make a good-faith effort to consult cvs blame in Bonsai to determine the Initial Developer of the code being moved.

If the moved code is all (or mostly) from one developer and you have not made substantial changes to the code when moving it, or have not made substantial additions (i.e., such that your code in the new file now comprises more of the file than the moved code), list the principal author of the moved code (or the author’s employer, if the author was in a work-for-hire situation) as the Initial Developer and as the first contributor (along with the “(Original Author)” tag). Add other developers of the moved code as contributors and then add yourself. If you have made substantial changes or additions to the code being moved, you should list yourself as the Initial Developer and Original Author in the Contributors section, and then include authors of the moved code as other Contributors.

Contributors to Existing Files

If you have made substantial additions or changes to a given file in a patch, or if, over time, you have done so, be sure to add yourself (along with your email address) to the “Contributor(s):” section of the license block.

Setting breakpoints in external files

Sometimes when debugging you will want to set breakpoints in files that are part of Camino, but not in the XCode project. For example, widget/src/cocoa/nsNativeThemeCocoa.mm. If you get an error about gdb not being able to find the file:linenumber, you will need to add the relevant shared library. Head to Run->Show->Shared Libraries... and set the Starting Level for the library to All. Which library to set it for depends on the file; for nsNativeThemeCocoa.mm, it is libwidget_mac.dylib. Turning off "Lazy Symbol Loading" in Debugger preferences may also help.

Preparing a patch to submit for review

XXXFIXME for hg commands

Before creating a patch for review, be sure that your source tree is up-to-date and that Camino builds with your changes.

In the $SRCDIR, cd mozilla/camino and then cvs up -dP. By default, cvs will attempt to merge any changes from the repository into your working copy. If it fails to merge the repository changes with your changes, cvs will inform you with a C mark before the file and a message about conflicts. You will need to resolve the conflict manually.

After updating your tree, switch back to $OBJDIR/camino and make (or build in Xcode) to make sure your changes and the latest code build. Ideally you would also launch Camino and test your changes one last time.

Proper patch format

Use cvs diff -u8N for patches to Camino code. Diffs should be done from $SRCDIR/mozilla/camino or $SRCDIR/mozilla for consistency and ease of application by reviewers and committers. All changes—including new source code files and project file changes, if applicable—should be submitted as a single patch (e.g., cvs diff -u8N src/foo/bar src/baz/bam > bugnumber.patch), with the exception of changes to nibs and images; see below for more about requirements for those three file types.

Adding new files in a patch

Sometimes the code you are writing requires the creation of new source files. In order to allow these files to be included in your single patch file, you must perform the additional step of cvs adding the file, or, if you do not have cvs write privileges, of imitating the cvs add process.

To make a patch including a new file (which otherwise requires write access to the repository), use the third-party cvsdo utility (which you'll need to install) to imitate the cvs add operation. cvsdo add each new file (alternatively add /Foo.h/0/dummy timestamp// to path/to/file/CVS/Entries), then diff as normal. If you do not include -N in your cvs diff command, the diff will fail.

To make a patch including files in new directories, first cvsdo add each new directory and then cvsdo add the files in those directories. Then create a diff of everything NOT in a new directory, and finally cvsdo diff on each new directory, appending the changes to your existing diff (cvsdo diff path/to/newdir/ >> EXISTING_DIFF). If you do not include -N in your cvs diff command, the diff will fail.

Removing files in a patch

Sometimes your patch obsoletes existing files in the tree. If you do not have write access to the repository, you will have to use the third-party cvsdo utility (which you'll need to install) to imitate the cvs remove operation. To indicate the file has been removed, first delete the file. Then cvsdo remove each removed file and diff as usual. If you do not include -N in your cvs diff command, the diff will fail.

Localizable.strings changes

Any UI string that appears in the code rather than in a nib needs to have an entry in Localizable.strings (or, in rare instances, a preference pane's Localizable.strings or one of the other .strings files). When writing new strings, always use “curly quotes” in the actual string text.

Beginning with Camino 1.6a1 (23-Oct-2007 nightlies), we have converted all .strings files to UTF-8 .strings.in files which can be diffed and which are then turned into UTF-16 .strings files in the build process. Any changes to .strings.in files should be included in your diff. Any new .strings files should be added to the tree as .strings.in files, converted to UTF-16 .strings files via the STRINGS_FILES mechanism in Makefile.in, and then add the resulting .strings files in generated/ to the project in the appropriate places.

To see string changes, you must rebuild Camino from the command line.

CAMINO_2_0_BRANCH

Baring any critical security bugs requiring .strings changes, the CAMINO_2_0_BRANCH is string frozen, and no string changes should occur there.

Project (Camino.xcodeproj) changes

Making project changes has to be done using Xcode (when adding, deleting, or moving files, as well as changing most build/compiler options), specifically when adding files. After making any project changes, simply diff the project file as well and include it in your patch (if you made any changes to build configuration or settings, be sure to diff camino/config/ as well to pick up xcconfig changes). Please check to ensure that your project patches do not change the state of the Camino.app entries in the Camino and CaminoStatic targets (this is usually a problem with srcdir configurations).

(Removing files can be done by hand if you don't have the proper version of Xcode, but this isn't recommended because removing the obvious entries for files will still leave non-obvious entries in the project file and cause crashes/build failures.)

Adding Gecko build products to Camino and the project file

• If the item is "optional" module (e.g., extension) and is not built by default, fix up confvars.sh (on the 1.8 branch, instead fix up the Camino section of configure.in and regenerate configure) to make Camino pull and build the item.
  • In some cases you can use an ac_add_options flag in your .mozconfig rather than patching confvars.sh/configure.in, but anything that's required to make Camino not burn when all is done needs to be declared in confvars.sh/configure.in.
• If the item required confvars.sh/configure.in or .mozconfig changes, do a full rebuild of your tree.
  • You'll typically have to build both non-static and static unless you're intimately familiar with the location of build products and naming conventions
• Add the libraries to appropriate build target(s), and using "Relative to Project" paths when adding
  • In most cases you'll want to add to the non-static (default) and static target separately
  • There are separate folders in the Xcode "sources" tree for static and non-static libs, and for components and "regular" libs
• Adding is best done from a srcdir build, but can be done with an objdir build if you carefully fix paths manually; note that in some cases Xcode will resolve symlinks when adding, so you'll have to fix the paths to point back to dist/*
• Move the products from Bundle Resources Copy Phase to the correct copy phase for that sort of Gecko product (for static builds, the static libs belong in the Libraries and Frameworks phase). Different versions of Xcode mis-guess the Copy Phase destinations in different ways.
• Repeat the previous two steps for any .xpt files that the libraries might require
• Check for, and remove if necessary, any bizarre changes to the header or library search paths that Xcode may have “helpfully” added for you; Xcode 3 typically adds garbage "QUOTED_SEARCH_PATHS_FOR_FOO" lines to the existing search paths and then declares that variable and sets it to the path of your newly-added library/header; you should delete these and add the library/header path to the existing search paths.
• Fix nsStaticComponents.cpp for static builds

Nib changes

When editing nibs, be sure to follow the guidelines in Development:Editing Nibs. Until further notice, all nib work should be done only in Interface Builder 2.5.x (preferably IB 2.5.4 under Mac OS X 10.4, as a number of features do not work properly in IB 2.5.6 under Mac OS X 10.5 and its nibs can load more slowly, but it can be run under in a pinch).

The Nib changes and other resource files section on the Reviewing page includes additional information about preparing your nib changes for review.

Images and other binary resources

Images should use the .tiff extension (with two “t"s) and be post-processed[1] with tiffutil. See Development:Preparing Graphics for full details.