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):

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

  if (murkyWaterFlowsUnderTheBridgeNearTheTallOakTree ||
      dirtyWaterRunsPastTheOldMillNearTheOpera)
  {
    return nil;
  }
  else {
    bop;
  }
}

When the condition is multiple lines, braces are required on the statement (with the opening brace on a line by itself). If the if statement had braces, the else statement must also have braces.

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.

As alluded to above, many of Camino’s older files have a mish-mash of styles. If you find particularly egregious cases, file bugs. You should not include unrelated whitespace or style cleanup in a patch; instead, you should perform cleanup in separate patches (or bugs) either before or after code changes. This keeps (blame for) code changes clear: if you are researching a section of code and hit a “mass style change” you can simply drop back to the previous revision and continue tracking the blame from there.

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 supports 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.

Testing your patch

Once you’ve written code and gotten it to compile, you need to test your patch to make sure it works (and works as expected). There are several methods available that allow you to launch another Camino at the same time as your daily-use Camino and/or protect your daily-use profile from development builds. Some methods may be more useful in certain situations than others.

See Development:Using Custom Profiles for detailed explanations of the various methods.

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

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 hg pull -u. By default, Mercurial will attempt to merge any changes from the repository into your working copy. If it fails to merge the repository changes with your changes, hg will inform you with a message about conflicts (and, depending on your configuration, dump you into your merge tool). You will need to resolve the conflict manually and then continue as instructed by Mercurial’s message.

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

You should have configured the proper patch format when setting up your Hg environment as part of the build instructions. Diffs should be done from $SRCDIR/mozilla/camino (or $SRCDIR/mozilla if you’re hacking on Gecko). All changes—including new source code files and project file changes, if applicable—should be submitted as a single patch (e.g., hg diff 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 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 hg adding the file before creating your patch.

Removing files in a patch

Sometimes your patch obsoletes existing files in the tree. To indicate the file has been removed, hg remove each file you want to remove and then diff as usual.

Moving, renaming, or copying files in a patch

Sometimes you want to move (or rename) a source file as part of your patch. As described in Helpful Mercurial Commands, you should always use hg mv to move/rename a file; you should never move or rename a file outside of Mercurial, because it will corrupt history.

If you want to copy a file, e.g., you are splitting a file into two pieces, or making a new file using much of the code in a given file and then changing some parts of it, use hg cp to create a copy of the original file with a new name/in its new location, and then make your changes to the files.

Once you have made your changes to the moved/renamed/copied files, you can diff as usual.

N.B. Showing file moves/renames or copies in patches requires you to have configured up your Hg environment to use “git-style” diffs as shown in the [diff] section of the Mercurial environment settings in the build instructions. To apply a patch that contains file moves/renames or copies, you must use hg import; see the Applying a patch with renames, copies, or binary files section of Helpful Mercurial Commands for the full syntax.

Localizable.strings changes

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

Editing .strings files

N.B All Camino .strings files exist in the tree as UTF-8 .strings.in files, since these files can be diffed and included in patches. Any changes to strings should be made to the .strings.in files, and these files should be included in your diff. (The build system later converts these files to the UTf-16 .strings files Cocoa requires.) To see string changes, you must rebuild Camino from the command line.

Adding new .strings files

If you need to add a new .strings file, create it in the tree as a UTF-8 .strings.in file and add it to the STRINGS_FILES block in Makefile.in (which will convert to the .strings.in file to a UTF-16 .strings files). Then rebuild Camino from the command line and add the resulting .strings files in generated/ to the project in the appropriate places.

String freezes

Branches are typically string-frozen (the string freeze usually coincides with the code freeze for the last beta milestone of a given release). 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). 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).

You should also verify that Xcode has not added “garbage” to any of the sections it modifies automatically, such as header or library search paths (this is most common with Gecko build products; see below).

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 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, but anything that is required to make Camino not burn when all is done needs to be declared in confvars.sh.
• If the item required confvars.sh or .mozconfig changes, do a full rebuild of your tree.
  • You will 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 libraries, and for components and “regular” libraries.
• 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 will 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 libraries 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 (or to the declarations in the .xcconfig, if applicable).
• Fix nsStaticComponents.cpp for static builds.

Nib changes

When editing nibs, be sure to follow the guidelines in Development:Editing Nibs. With the exception of FindBarTextured, all nibs should be saved in the Xcode 2.x-compatible format. Performance-critical nibs (BrowserWindow, MainMenu) should be saved 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 used in a pinch). If you are working on a performance-critical nib and do not have Xcode 2.5/IB 2.5, make your changes anyway but add a comment when uploading the nib that it will need to be resaved in IB 2.5.

The Nib changes and other resource files section on the Reviewing page includes additional information about preparing your nib changes for review and attaching them to bugs. Do not include nibs or other binary resource files as part of a git-style Hg patch.

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.

The Nib changes and other resource files section on the Reviewing page includes additional information about preparing your images for review and attaching them to bugs. Do not include nibs or other binary resource files as part of a git-style Hg patch.