Closed Bug 779473 (nsresult-enum-class) Opened 12 years ago Closed 12 years ago

Make nsresult an enum class

Categories

(Core :: XPCOM, enhancement)

Product:
Core
Component:
XPCOM
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla19

People

(Reporter: ayg, Assigned: ayg)

References

Details

(Keywords: dev-doc-complete)

Attachments

(4 files)

Patch
96.65 KB, patch
ehsan.akhgari
: review+
Details | Diff | Splinter Review
readelf -WS of editor/libeditor/base/EditTxn.o without all the extra consts
10.81 KB, text/plain
Details
readelf -WS of editor/libeditor/base/EditTxn.o without all the extra consts
55.11 KB, text/plain
Details
Interdiff to use #define instead of static consts (to be folded)
3.54 KB, patch
ehsan.akhgari
: review+
Details | Diff | Splinter Review 
Bug 777292 is now solved for m-c, although there are plenty of patches still to land, so I'm going to move on to new territory: make nsresult an enum *class*!  This will prevent implicit conversions not only to nsresult, but from it as well, so things like
  bool foo() { return NS_OK; }
or "if (res)" won't compile.  Enum class support in Gecko for old compilers is bug 751554.

It's been suggested we make nsresult a class instead.  Why?  It would work somewhat better for old compilers, but I see no advantage for newer compilers (gcc 4.4, Clang 2.9, VC11), and defining it would be considerably more complicated.  (The enum class fallback in bug 751554 provides some of the type safety of an actual enum class, but not all.)

Having learned from the last bug, I'll file all breakage fixes as separate bugs, and keep this bug only for the enum class implementation itself.
. . . so slight problem here: how do I deal with scoping?  Just making nsresult an enum class means you have to use nsresult::NS_OK instead of just NS_OK, which obviously is not acceptable.  My workaround for now is to copy and paste all the enum codes, like

const nsresult NS_OK = nsresult::NS_OK,
               NS_ERROR_BASE = nsresult::NS_ERROR_BASE,
               NS_ERROR_NOT_INITIALIZED = nsresult::NS_ERROR_NOT_INITIALIZED,
               ...

but that's 424 lines and annoying to maintain.  I can't think of any cleverer way to do it, though.
If you use a plain class, you could keep an nsresult_enum that is the "current" enum and add an assignment operator to the class, no?
(In reply to Rafael Ávila de Espíndola (:espindola) from comment #2)
> If you use a plain class, you could keep an nsresult_enum that is the
> "current" enum and add an assignment operator to the class, no?

Sure, but what's the gain?  In particular, we'd still want nsresult_enum to be an enum class rather than a regular enum, to prevent things like "int foo() { return NS_OK; }".  In that setup, the named constants will still be of the enum type, not a class.  That adds complexity, so why do it?

(You'd need a conversion operator, not just an assignment operator, so that returning an nsresult_enum from a function with return type nsresult still worked, and suchlike.  This approach is basically workable, and is how the fallback for MOZ_*_ENUM_CLASS works in my patch in bug 751554.  I just don't see why it's useful here.)
nsresult_enum would only be used to implement nsresult, so you would still get all the type safety in user code that would be using the nsresult class.
(In reply to Rafael Ávila de Espíndola (:espindola) from comment #4)
> nsresult_enum would only be used to implement nsresult, so you would still
> get all the type safety in user code that would be using the nsresult class.

Sorry, not all, you would still get the enum constant -> int conversion, that is true.
Depends on: 779809
(In reply to comment #1)
> . . . so slight problem here: how do I deal with scoping?  Just making nsresult
> an enum class means you have to use nsresult::NS_OK instead of just NS_OK,
> which obviously is not acceptable.  My workaround for now is to copy and paste
> all the enum codes, like
> 
> const nsresult NS_OK = nsresult::NS_OK,
>                NS_ERROR_BASE = nsresult::NS_ERROR_BASE,
>                NS_ERROR_NOT_INITIALIZED = nsresult::NS_ERROR_NOT_INITIALIZED,
>                ...
> 
> but that's 424 lines and annoying to maintain.  I can't think of any cleverer
> way to do it, though.

Here's what I would suggest.  Add a new file named something like nsErrorList.h and use macros like this:

NSRESULT_ERROR_VALUE(NS_OK, 0)
NSRESULT_ERROR_VALUE(NS_ERROR_FOO, NS_GENERATE_ERROR(...))
...

Then, in the enum class body, do:

#define NSRESULT_ERROR_VALUE(name, value) name = value,
#include "nsErrorList.h"
#undef NSRESULT_ERROR_VALUE

And after the class you could do:

#define NSRESULT_ERROR_VALUE(name, value) static const name = nsresult::name;
#include "nsErrorList.h"
#undef NSRESULT_ERROR_VALUE

That way, we only need to maintain the global list of errors in nsErrorList.h, and we can make the NSRESULT_ERROR_VALUE do different things depending on who includes it.

Does that address the problem sufficiently?
Right -- excellent point!  I had seen that pattern before a bunch, but didn't think to use it.  Thanks!
(In reply to comment #7)
> Right -- excellent point!  I had seen that pattern before a bunch, but didn't
> think to use it.  Thanks!

No problem!  I will buy a coffee for whoever can tell me the name of this pattern so that I don't have to type it in fully the next time I want to mention it.  :-)
You can buy one for Kim Gräsman, who gave a reference that calls it "X Macros":

http://code.google.com/p/include-what-you-use/issues/detail?id=74#c3
http://www.drdobbs.com/the-new-c-x-macros/184401387

It's not a very descriptive name, but it is a name.  This page calls it "supermacros", which isn't a lot better:

http://wanderinghorse.net/computing/papers/supermacros_cpp.html

How about you make up a better name and write a blog post about it so other people can link to it?  Maybe it will catch on!  :)
Depends on: 782252
(In reply to comment #9)
> You can buy one for Kim Gräsman, who gave a reference that calls it "X Macros":
> 
> http://code.google.com/p/include-what-you-use/issues/detail?id=74#c3
> http://www.drdobbs.com/the-new-c-x-macros/184401387
> 
> It's not a very descriptive name, but it is a name.  This page calls it
> "supermacros", which isn't a lot better:
> 
> http://wanderinghorse.net/computing/papers/supermacros_cpp.html
> 
> How about you make up a better name and write a blog post about it so other
> people can link to it?  Maybe it will catch on!  :)

I'm not very good at naming things, but I can write a blog post about this for sure!  :-)
Depends on: 782594
Depends on: 782605
Depends on: 782606
Depends on: 782608
Depends on: 782614
Depends on: 782616
Depends on: 782622
Depends on: 782916
Depends on: 782919
It looks like our enum class workaround for non-supporting compilers doesn't work for nsresult.  If you do

  static const nsresult NS_OK = nsresult::NS_OK;

then nsresult::NS_OK is of type nsresult::Enum, which is an enum, but NS_OK is of type nsresult, which is a class.  Thus NS_OK can no longer be used as a switch case unless you make it constexpr, which isn't supported by all of the compiler versions we care about.  (In particular, no version of VS supports it.)

On the other hand, if you do

  static const nsresult::Enum NS_OK = nsresult::NS_OK;

then expressions like

  return NS_SUCCEEDED(res) ? res : NS_ERROR_FAILURE;

fail because the two last arguments to the ternary operator have different type, and either one can be converted to the other, so it's ambiguous.  The C++ standard unfortunately does not make compilers figure out that the eventual returned value is the same either way.

So it looks like I'll have to check for HAVE_CXX11_STRONG_ENUMS explicitly and fall back to a regular enum otherwise (which we have to do for C anyway).
No longer depends on: 782919
No longer depends on: 782594
No longer depends on: 782616
Alias: nsresult-enum-class
Depends on: 783869
Depends on: 791213
Depends on: 791216
Depends on: 794905
Bug 779809 is now on inbound, and that was the last blocker.  Bug 794905 has cropped up too since my last compile check, but the patch is trivial and I don't foresee problems getting it through.  Try run to see if there are any other problems:

https://tbpl.mozilla.org/?tree=Try&rev=84eb6d476b98


Important note: when doing a debug build locally, on 32-bit PAE Linux with gcc 4.6.3 and 16G RAM, linking fails with

  /usr/bin/ld.bfd.real: failed to set dynamic section sizes: Memory exhausted
  collect2: ld returned 1 exit status

In principle, it should have 4G of address space, due to PAE.  This is pretty strange -- perhaps there's some bug in gcc/ld that requires lots more address space for enum classes than for enums.  I Googled it and found nothing interesting, which isn't surprising, because this will only come up with really large projects and probably they don't use enum class yet.  Ehsan, any thoughts on what to do about this?

As a workaround, we could conceivably disable nsresult-as-enum-class on gcc if that's the only compiler that exhibits the problem.  (I guess this must be gcc-specific, because ld doesn't know about the difference between enum and enum class.)
Attached patch Patch — — Splinter Review 
I figured it was about time to post this for review, even though it's not quite ready to land.

The << overload in storage_test_harness.h is because it passes nsresult to an ostream in a macro.  Previously it would get cast implicitly to some numeric type (hopefully the underlying type!).

I moved NS_ERROR_GET_CODE etc. down below the enum definition and turned them into type-safe inline functions.  I think there's some reason I had to do this to get it to compile, but I can't remember what.  :(  Should be harmless anyway, they're not exactly perf-sensitive.

Should I change the uint32_t()-style casts to static_cast<uint32_t>(), for consistency?  I just noticed that as I reviewed the patch for submission.
Attachment #665428 - Flags: review?(ehsan)
The linking problem only occurs for debug builds.  So maybe this is the culprit:

  /* Define unscoped versions too: NS_OK = nsresult::NS_OK, etc. */
  #define ERROR(key, val) key = nsresult::key
  static const nsresult
  #include "nsErrorList.h"
  ;

That's putting 424 or so extra symbols in the symbol table for every single compilation unit that includes nsError.h?  Might that be the issue?  (Although you'd think a typical compilation unit would have many more symbols than that anyway, so I don't know why it's such a big deal.)  Unfortunately, macros can't themselves define macros, so I don't see a way out.

We were discussing this a bunch on #developers.  Really what we need is some way to get the compiler to either not store the symbols (like by using the preprocessor), or consolidate the symbols it does store (like by using external linkage).  But nothing we hit upon seems to solve the problem adequately.
(In reply to :Aryeh Gregor from comment #14)
> The linking problem only occurs for debug builds.  So maybe this is the
> culprit:
> 
>   /* Define unscoped versions too: NS_OK = nsresult::NS_OK, etc. */
>   #define ERROR(key, val) key = nsresult::key
>   static const nsresult
>   #include "nsErrorList.h"
>   ;
> 
> That's putting 424 or so extra symbols in the symbol table for every single
> compilation unit that includes nsError.h?  Might that be the issue? 
> (Although you'd think a typical compilation unit would have many more
> symbols than that anyway, so I don't know why it's such a big deal.) 
> Unfortunately, macros can't themselves define macros, so I don't see a way
> out.
> 
> We were discussing this a bunch on #developers.  Really what we need is some
> way to get the compiler to either not store the symbols (like by using the
> preprocessor), or consolidate the symbols it does store (like by using
> external linkage).  But nothing we hit upon seems to solve the problem
> adequately.

Hmm, I've never come across this problem before...  Maybe Rafael has an idea?
Comment on attachment 665428 [details] [diff] [review]
Patch

Review of attachment 665428 [details] [diff] [review]:
-----------------------------------------------------------------

::: xpcom/base/nsError.h
@@ +9,5 @@
>  #ifndef nscore_h___
>  #include "nscore.h"  /* needed for nsresult */
>  #endif
>  
> +#if defined(__cplusplus) && !defined(mozilla_Attributes_h_)

Why the mozilla_Attributes_h_ check?  Can't you just include this #ifdef __cplusplus?

@@ +124,5 @@
> +#if defined(__cplusplus) && defined(MOZ_HAVE_CXX11_STRONG_ENUMS)
> +  typedef enum class tag_nsresult : uint32_t
> +  {
> +    #define ERROR(key, val) key = val
> +    #include "nsErrorList.h"

Hmm, I would drop the trailing comma from the ERROR macros in nsErrorList.h, and would include that in the #define's here and below.  In the future we might need to use that file in constructs which do not require the trailing comma.

Also, please name this file ErrorList.h.  :-)
Attachment #665428 - Flags: review?(ehsan) → review+
Can we compare |readelf -WS| on some compilation unit with and without enum classes?

I don't think there's a good way to get "constant definitions that the compiler is willing to optimize away at -O0" without resorting to the preprocessor.
(In reply to Ehsan Akhgari [:ehsan] from comment #16)
> Why the mozilla_Attributes_h_ check?  Can't you just include this #ifdef
> __cplusplus?

I assume the theory is that this way the preprocessor doesn't even have to load the contents of the header (which will then be skipped over completely) -- it doesn't change semantics at all.  So if some other header already loaded Attributes.h, nsError.h won't have to load it again.  I dunno if this is actually worth it, but I figured I'd go with what the other includes in the file do.

> > +#if defined(__cplusplus) && defined(MOZ_HAVE_CXX11_STRONG_ENUMS)
> > +  typedef enum class tag_nsresult : uint32_t
> > +  {
> > +    #define ERROR(key, val) key = val
> > +    #include "nsErrorList.h"
> 
> Hmm, I would drop the trailing comma from the ERROR macros in nsErrorList.h,
> and would include that in the #define's here and below.  In the future we
> might need to use that file in constructs which do not require the trailing
> comma.

I originally did it that way, but this is a syntax error:

  static const nsresult foo = bar,
                        baz = quz,
  ;

so I can't.  Unless you'd like me to use a different macro for the last item, like LAST_ERROR, and define two, but that seems like it would be appropriate to do only once we need it.

(In an enum, a trailing comma after the last item is allowed, but 
IIUC only by the most recent standards, so my version of gcc warns about it with -pedantic -- which is also a problem with warnings-as-errors on.  This will presumably be fixed in future gcc versions, so it's not as big an issue.)

> Also, please name this file ErrorList.h.  :-)

Done!

(In reply to Nathan Froyd (:froydnj) from comment #17)
> Can we compare |readelf -WS| on some compilation unit with and without enum
> classes?

I'll upload some.

> I don't think there's a good way to get "constant definitions that the
> compiler is willing to optimize away at -O0" without resorting to the
> preprocessor.

This isn't compatible with the way I've got ErrorList.h set up, though, right?  I can't have a #define do a #define itself.  Given that, is there any reasonable way to do this using the preprocessor alone without copy-pasting, or do I have to write a little Python script or something to generate two distinct headers to give to the preprocessor?
This is a very small file, so it has only 108 section headers.  I checked a bigger file and it had a few thousand.
Same file, but now with 532 section headers, as expected.  I still don't get why this should make my linker run out of address space, though, particularly since I'm using PAE, and it builds fine on try.
(In reply to :Aryeh Gregor from comment #18)
> This isn't compatible with the way I've got ErrorList.h set up, though,
> right?  I can't have a #define do a #define itself.  Given that, is there
> any reasonable way to do this using the preprocessor alone without
> copy-pasting, or do I have to write a little Python script or something to
> generate two distinct headers to give to the preprocessor?

This doesn't sound like a terrible idea, honestly. If the preprocessor is preventing you from doing the right thing here, implementing your own code generator is a sane idea.
(In reply to :Aryeh Gregor from comment #20)
> Created attachment 665841 [details]
> readelf -WS of editor/libeditor/base/EditTxn.o without all the extra consts
> 
> Same file, but now with 532 section headers, as expected.  I still don't get
> why this should make my linker run out of address space, though,
> particularly since I'm using PAE, and it builds fine on try.

These sections have increased in size:

.debug_info
.rel.debug_info
.shstrtab
.symtab
.strtab

for a total increase of 60K, maybe a bit more once you take into account the extra space all those constants require.

My debug build has 5K object files, so that's roughly 300M of extra data to take in.  Then there's some expansion of that because you need pointers into the data, heavier representations of various bits, and so forth.  It's not unreasonable to think there's ~500M of extra data to churn through, and if you're already close to the 4GB limit...
(In reply to comment #18)
> (In reply to Ehsan Akhgari [:ehsan] from comment #16)
> > Why the mozilla_Attributes_h_ check?  Can't you just include this #ifdef
> > __cplusplus?
> 
> I assume the theory is that this way the preprocessor doesn't even have to load
> the contents of the header (which will then be skipped over completely) -- it
> doesn't change semantics at all.  So if some other header already loaded
> Attributes.h, nsError.h won't have to load it again.  I dunno if this is
> actually worth it, but I figured I'd go with what the other includes in the
> file do.

That may shave off a few nanoseconds, but compilers usually optimize this case anyway, and it's not really worth it.

> > > +#if defined(__cplusplus) && defined(MOZ_HAVE_CXX11_STRONG_ENUMS)
> > > +  typedef enum class tag_nsresult : uint32_t
> > > +  {
> > > +    #define ERROR(key, val) key = val
> > > +    #include "nsErrorList.h"
> > 
> > Hmm, I would drop the trailing comma from the ERROR macros in nsErrorList.h,
> > and would include that in the #define's here and below.  In the future we
> > might need to use that file in constructs which do not require the trailing
> > comma.
> 
> I originally did it that way, but this is a syntax error:
> 
>   static const nsresult foo = bar,
>                         baz = quz,
>   ;
> 
> so I can't.  Unless you'd like me to use a different macro for the last item,
> like LAST_ERROR, and define two, but that seems like it would be appropriate to
> do only once we need it.

Shoot!  No this is fine then.

> (In an enum, a trailing comma after the last item is allowed, but 
> IIUC only by the most recent standards, so my version of gcc warns about it
> with -pedantic -- which is also a problem with warnings-as-errors on.  This
> will presumably be fixed in future gcc versions, so it's not as big an issue.)

Yes, tributes to the designers of C. :(

> > I don't think there's a good way to get "constant definitions that the
> > compiler is willing to optimize away at -O0" without resorting to the
> > preprocessor.
> 
> This isn't compatible with the way I've got ErrorList.h set up, though, right? 
> I can't have a #define do a #define itself.  Given that, is there any
> reasonable way to do this using the preprocessor alone without copy-pasting, or
> do I have to write a little Python script or something to generate two distinct
> headers to give to the preprocessor?

Doing this in a python script may not be a terrible idea in fact!  :-)
Things we could try:

* Using a new bfd ld
* Using gold
* Only enabling -ffunction-sections -fdata-sections on non debug builds

The last item is probably a good thing anyway and should be a simple change to compiler-opts.m4.
(In reply to Ehsan Akhgari [:ehsan] from comment #23)
> That may shave off a few nanoseconds, but compilers usually optimize this
> case anyway, and it's not really worth it.

Okay, I'll remove that bit.

> Doing this in a python script may not be a terrible idea in fact!  :-)

I guess I'll go with that.
Depends on: 799892
So bug 792188 added another place that passes nsresult to a std::ostream.  I tried moving the nsresult overload for << to nsError.h, without pulling in anything more than iosfwd, but eventually got

  ../../dist/stl_wrappers/cwchar:36:4: error: #error "STL code can only be used with infallible ::operator new()"

which I don't know how to deal with.  Is there any nice way to do this, or should we just require anyone who wants to do this to static_cast<uint32_t> it?
(In reply to comment #26)
> So bug 792188 added another place that passes nsresult to a std::ostream.  I
> tried moving the nsresult overload for << to nsError.h, without pulling in
> anything more than iosfwd, but eventually got
> 
>   ../../dist/stl_wrappers/cwchar:36:4: error: #error "STL code can only be used
> with infallible ::operator new()"
> 
> which I don't know how to deal with.  Is there any nice way to do this, or
> should we just require anyone who wants to do this to static_cast<uint32_t> it?

We can't use STL headers in some places in our code, including code built with NOXPCOM (don't remember what the actual flag name is) and nsError.h is included in those files...  So, any solution which requires pulling in STL headers in nsError.h will fail.  We should just require people static_cast their way out of this I think.
Depends on: 800343
froydnj advised me on how to do some of the Makefile stuff.  It turns out a script is overkill -- sed is fine.  I checked the OS X man page for sed and it looks like the relevant features are supported.  I got "-n" and "p" off Google, and documented them because they're not so well-known (at least I didn't know about them).

This approach means that it's impossible to actually use the full names anywhere in code -- "nsresult::NS_OK" will expand to "nsresult::nsresult::NS_OK" and not work.  This might be confusing if a debugger or something helpfully expands the macros for you.  I tried working around it by stripping the "NS_" prefix, so it was "#define NS_OK nsresult::OK", but it broke on some file that (I think, judging by the compiler error) #define'd OK to be something else, so I figured it wasn't worth it and gave up.
Attachment #670422 - Flags: review?(ehsan)
Could we not generate both a .cpp and a .h, and have the .h just have extern definitions and the .cpp contain actual definitions, and use consts in the .cpp?
(In reply to Ted Mielczarek [:ted] from comment #29)
> Could we not generate both a .cpp and a .h, and have the .h just have extern
> definitions and the .cpp contain actual definitions, and use consts in the
> .cpp?

Then you get memory references in all the places you use error codes, unless LTO comes and saves you.
Comment on attachment 670422 [details] [diff] [review]
Interdiff to use #define instead of static consts (to be folded)

Review of attachment 670422 [details] [diff] [review]:
-----------------------------------------------------------------

::: xpcom/base/Makefile.in
@@ +163,5 @@
> +# We generate ErrorListDefines.h from ErrorList.h using regex.  The -n option
> +# suppresses printing the pattern space, and the p at the end prints it anyway,
> +# so we don't print lines that don't match the pattern to start with.
> +ErrorListDefines.h: ErrorList.h
> +		sed -n 's/.*ERROR(\([A-Z_0-9]*\).*/#define \1 nsresult::\1/p' < $< > $@

Honestly, I can't read the sed magic going on here, but if it works, it's fine by me!
Attachment #670422 - Flags: review?(ehsan) → review+
Okay, so here's a new all-platform build-only try:

https://tbpl.mozilla.org/?tree=Try&rev=d8dd96e22734

If that turns up green, I'll push to inbound.
(In reply to comment #32)
> Okay, so here's a new all-platform build-only try:
> 
> https://tbpl.mozilla.org/?tree=Try&rev=d8dd96e22734
> 
> If that turns up green, I'll push to inbound.

One more brokenness added in BluetoothService.cpp.  Please go ahead and fix it, and r=me if it's just using the right type.  Thanks so much for keeping on working this! :-)
Try run for d10dcfbf73c9 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=d10dcfbf73c9
Results (out of 16 total builds):
    exception: 10
    failure: 6
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-d10dcfbf73c9
Try run for 39f14f0ae7f1 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=39f14f0ae7f1
Results (out of 16 total builds):
    exception: 13
    failure: 3
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-39f14f0ae7f1
Try run for a6c6124315fc is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=a6c6124315fc
Results (out of 16 total builds):
    success: 13
    failure: 3
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-a6c6124315fc
Try run for 3847c7fb8ebc is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=3847c7fb8ebc
Results (out of 16 total builds):
    exception: 8
    success: 1
    failure: 7
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-3847c7fb8ebc
Try run for 3847c7fb8ebc is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=3847c7fb8ebc
Results (out of 18 total builds):
    exception: 8
    success: 3
    failure: 7
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-3847c7fb8ebc
https://hg.mozilla.org/mozilla-central/rev/5c273dc49dee
https://hg.mozilla.org/mozilla-central/rev/169ae9c542e8
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Blocks: 801383
Depends on: 801472
Depends on: 801519
Depends on: 801579
Depends on: 801589
No longer depends on: 801579
Depends on: 801471
Depends on: 802884
Depends on: 901200
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: