Difference between revisions of "Development:Coding"
(→Nib changes: move the review-related stuff back to Reviewing; add images) |
(→Images and other binary resources: i knew we had a page on this....) |
||
Line 111: | Line 111: | ||
=== Images and other binary resources === | === Images and other binary resources === | ||
− | Images should use the <tt>.tiff</tt> extension (with two “t"s) and be post-processed[https://bugzilla.mozilla.org/show_bug.cgi?id=384725#c13] with <tt>tiffutil</tt>. | + | Images should use the <tt>.tiff</tt> extension (with two “t"s) and be post-processed[https://bugzilla.mozilla.org/show_bug.cgi?id=384725#c13] with <tt>tiffutil</tt>. See [[Development:Preparing Graphics]] for full details. |
− | |||
− | |||
− | |||
− | |||
− | |||
− |
Revision as of 23:16, 14 November 2008
Here are some useful tips for coding a feature or bug fix for Camino and creating a patch to send your changes through review.
Contents
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.
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, 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 alsos 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
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 cvs up. 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.
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)
Removing files in a patch
Sometimes your patch obsoletes existing files in the tree.
Instructions TBA
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.
MOZILLA_1_8_BRANCH
Baring any critical security bugs requiring .strings changes, the MOZILLA_1_8_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 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
- If the library or .xpt is something that seems relevant to all embeddors, embedding/config/basebrowser-mac-macho should probably be modified to deliver the build products to dist/Embed/* rather than dist/*
- 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 anobjdir
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/Embed/* (or dist/*, if appropriate) - 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.
- 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.