Add error category to SSL error messages sent to Browser Console (aka. Error Console)

RESOLVED FIXED in mozilla27

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: patilkr24, Assigned: patilkr24)

Tracking

(Blocks 2 bugs)

unspecified
mozilla27
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 11 obsolete attachments)

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/28.0.1500.71 Chrome/28.0.1500.71 Safari/537.36

Steps to reproduce:

SSL error messages are sent to Browser console as string messages. They are not instance of nsIScriptError and does not include error category in the message such as "SSL" or "Secure Socket Layer" category should be associated with the SSL errors or warning generated by browser. 



Expected results:

SSL errors should have error category associated with them.
Blocks: 863874, 890224
Component: Untriaged → Security
OS: Linux → All
Hardware: x86_64 → All
Posted patch bug-898712.patch (obsolete) — Splinter Review
Patch adds "SSL" category to SSL errors and creates them an instance of nsIScriptError.
Attachment #788212 - Flags: feedback?(tanvi)
Attachment #788212 - Flags: feedback?(mgoodwin)
Posted patch bug-898712.patch (obsolete) — Splinter Review
Patch adds "SSL" category to SSL errors and creates them an instance of nsIScriptError.
Attachment #788212 - Attachment is obsolete: true
Attachment #788212 - Flags: feedback?(tanvi)
Attachment #788212 - Flags: feedback?(mgoodwin)
Attachment #788476 - Flags: feedback?(tanvi)
Attachment #788476 - Flags: feedback?(mgoodwin)
How can I change status (UNCONFIRMED) of this bug ? Status drop-down box only shows me two options: UNCONFIRMED and RESOLVED.
(In reply to Kailas from comment #3)
> How can I change status (UNCONFIRMED) of this bug ? Status drop-down box
> only shows me two options: UNCONFIRMED and RESOLVED.

Not sure why you are seeing this, but I've changed the status to NEW and assigned this bug to you.
Assignee: nobody → patilkr24
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment on attachment 788476 [details] [diff] [review]
bug-898712.patch

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

Fix the whitespace issue and it's OK with me. You'll want review from grobinson or someone, I expect.

::: security/manager/ssl/src/SSLServerCertVerification.cpp
@@ +226,5 @@
> +                       EmptyString(), 
> +                       EmptyString(), 
> +                       0, 
> +                       0, 
> +                       nsIScriptError::errorFlag, 

Nit: All args to Init so far have trailing white space.
Attachment #788476 - Flags: feedback?(mgoodwin) → feedback+
Posted patch bug-898712.patch (obsolete) — Splinter Review
Trailing white space removed from all args of Init
Attachment #790344 - Flags: review?(grobinson)
Comment on attachment 790344 [details] [diff] [review]
bug-898712.patch

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

drive by review.

::: security/manager/ssl/src/SSLServerCertVerification.cpp
@@ +217,5 @@
>    
>    if (!message.IsEmpty()) {
>      nsCOMPtr<nsIConsoleService> console;
>      console = do_GetService(NS_CONSOLESERVICE_CONTRACTID);
> +    nsCOMPtr<nsIScriptError> error;

I would rename this variable.. error is too generic in a function that handles errors. Better scriptErr (?) I am wide open to suggestions.

@@ +220,5 @@
>      console = do_GetService(NS_CONSOLESERVICE_CONTRACTID);
> +    nsCOMPtr<nsIScriptError> error;
> +    error = do_CreateInstance(NS_SCRIPTERROR_CONTRACTID);
> +
> +    if (console && error) {

This block is hard to read. I think it is clrearer to have it:

if(console) {
  nsCOMPtr<nsIScriptError> error;
  scriptError = do_CreateInstance(NS_SCRIPTERROR_CONTRACTID);
  if (scriptError) {
    rv=scrptError->Init....
  }
  else {
    console->Log.....
  }
}
(In reply to Camilo Viecco (:cviecco) from comment #7)
> Comment on attachment 790344 [details] [diff] [review]
> bug-898712.patch
> 
> Review of attachment 790344 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> drive by review.
> 
> ::: security/manager/ssl/src/SSLServerCertVerification.cpp
> @@ +217,5 @@
> >    
> >    if (!message.IsEmpty()) {
> >      nsCOMPtr<nsIConsoleService> console;
> >      console = do_GetService(NS_CONSOLESERVICE_CONTRACTID);
> > +    nsCOMPtr<nsIScriptError> error;
> 
> I would rename this variable.. error is too generic in a function that
> handles errors. Better scriptErr (?) I am wide open to suggestions.
> 
How about sslScriptErr?

> @@ +220,5 @@
> >      console = do_GetService(NS_CONSOLESERVICE_CONTRACTID);
> > +    nsCOMPtr<nsIScriptError> error;
> > +    error = do_CreateInstance(NS_SCRIPTERROR_CONTRACTID);
> > +
> > +    if (console && error) {
> 
> This block is hard to read. I think it is clrearer to have it:
> 
> if(console) {
>   nsCOMPtr<nsIScriptError> error;
>   scriptError = do_CreateInstance(NS_SCRIPTERROR_CONTRACTID);
>   if (scriptError) {
>     rv=scrptError->Init....
>   }
>   else {
>     console->Log.....
>   }
> }
Posted patch bug-898712.patch1 (obsolete) — Splinter Review
cviecco: updated patch according to your suggestions. Thanks!
I created this patch on hg changeset:142619:a8daa428ccbc
Attachment #790344 - Attachment is obsolete: true
Attachment #790344 - Flags: review?(grobinson)
Attachment #790634 - Flags: review?(cviecco)
Comment on attachment 790634 [details] [diff] [review]
bug-898712.patch1

>diff -r a8daa428ccbc security/manager/ssl/src/SSLServerCertVerification.cpp
>--- a/security/manager/ssl/src/SSLServerCertVerification.cpp	Wed Aug 14 17:04:38 2013 -0400
>+++ b/security/manager/ssl/src/SSLServerCertVerification.cpp	Thu Aug 15 09:41:08 2013 +0530
>@@ -116,6 +116,8 @@
> #include "nsIConsoleService.h"
> #include "PSMRunnable.h"
> #include "SharedSSLState.h"
>+#include "nsError.h"
>+#include "nsIScriptError.h"
> 
> #include "ssl.h"
> #include "secerr.h"
>@@ -210,13 +212,29 @@
>                     nsIX509Cert* ix509)
> {
>   nsString message;
>+  nsresult rv;
>   socketInfo->GetErrorLogMessage(errorCode, errorMessageType, message);
>   
>   if (!message.IsEmpty()) {
>     nsCOMPtr<nsIConsoleService> console;
>     console = do_GetService(NS_CONSOLESERVICE_CONTRACTID);
>     if (console) {
>-      console->LogStringMessage(message.get());
>+      nsCOMPtr<nsIScriptError> scriptError;
>+      scriptError = do_CreateInstance(NS_SCRIPTERROR_CONTRACTID);
>+      if (scriptError) {
>+        rv = scriptError->Init(message,
>+                         EmptyString(),
>+                         EmptyString(),
>+                         0,
>+                         0,
>+                         nsIScriptError::errorFlag,
>+                         "SSL");
"SSL" is the category you are using.  You shoudl add that category here so that it gets mapped to CATEGORY_SECURITY instead of the default CATEGORY_JS:
https://mxr.mozilla.org/mozilla-central/source/browser/devtools/webconsole/webconsole.js#4533 


>+        if (NS_SUCCEEDED(rv)) {
>+          console->LogMessage(scriptError);
>+          return;
>+        }
>+      }
>+       console->LogStringMessage(message.get());
>     }
>   }
> }
Attachment #790634 - Flags: feedback+
Comment on attachment 788476 [details] [diff] [review]
bug-898712.patch

Should this patch be marked as obsolete?
Attachment #788476 - Flags: feedback?(tanvi)
drive by review:

diff -r a8daa428ccbc security/manager/ssl/src/SSLServerCertVerification.cpp
--- a/security/manager/ssl/src/SSLServerCertVerification.cpp	Wed Aug 14 17:04:38 2013 -0400
+++ b/security/manager/ssl/src/SSLServerCertVerification.cpp	Thu Aug 15 09:41:08 2013 +0530

+        rv = scriptError->Init(message,
+                         EmptyString(),
+                         EmptyString(),
+                         0,
+                         0,
+                         nsIScriptError::errorFlag,
+                         "SSL");

Please align all consecutive arguments with the first(message) argument. 
+      }
+       console->LogStringMessage(message.get());

console is indented one extra space here.
Product: Firefox → Core
Posted patch bug-898712.patch (obsolete) — Splinter Review
removed extra space and uploaded updated patch.
Attachment #788476 - Attachment is obsolete: true
Attachment #790634 - Attachment is obsolete: true
Attachment #790634 - Flags: review?(cviecco)
Attachment #791131 - Flags: review?(tanvi)
Attachment #791131 - Flags: review?(cviecco)
Attachment #791131 - Flags: review?(alagenchev)
(In reply to Tanvi Vyas [:tanvi] from comment #11)
> Comment on attachment 788476 [details] [diff] [review]
> bug-898712.patch
> 
> Should this patch be marked as obsolete?

Yes. Done
Hi Kailas, did you upload the right patch? It doesn't appear that you addressed the issues that we brought up above.
Attachment #791131 - Flags: review?(alagenchev)
Also, tanvi, cviecco and I aren't module peers for the module. You should feedback? us and r? at the end once we are done with our feedback. The people that you can request r+ from for each module are listed here: https://wiki.mozilla.org/Modules/All#Submodules. This module is under Security - Mozilla PSM Glue.
Posted patch bug-898712.patch (obsolete) — Splinter Review
updated patch.
Attachment #791131 - Attachment is obsolete: true
Attachment #791131 - Flags: review?(tanvi)
Attachment #791131 - Flags: review?(cviecco)
Attachment #791394 - Flags: feedback?(tanvi)
Attachment #791394 - Flags: feedback?(cviecco)
Attachment #791394 - Flags: feedback?(alagenchev)
Attachment #791394 - Flags: feedback?(tanvi) → feedback+
Comment on attachment 791394 [details] [diff] [review]
bug-898712.patch

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

r+ With comments addressed

::: security/manager/ssl/src/SSLServerCertVerification.cpp
@@ +211,5 @@
>                      ::mozilla::psm::SSLErrorMessageType errorMessageType,
>                      nsIX509Cert* ix509)
>  {
>    nsString message;
> +  nsresult rv;

minor issue. Move this declaration to the block where you actually use it.
Attachment #791394 - Flags: feedback?(cviecco) → feedback+
Attachment #791394 - Flags: feedback?(alagenchev) → feedback+
Posted patch bug-898712.patch (obsolete) — Splinter Review
cviecco: comments addressed. 
Thanks to all for your valuable feedback. I will avoid those mistakes in future patches.
Attachment #791394 - Attachment is obsolete: true
Attachment #791594 - Flags: review?(tanvi)
Attachment #791594 - Flags: review?(cviecco)
Attachment #791594 - Flags: review?(brian)
Comment on attachment 791594 [details] [diff] [review]
bug-898712.patch

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

Is there a way to add a test for this feature?
Attachment #791594 - Flags: review?(cviecco)
(In reply to Camilo Viecco (:cviecco) from comment #20)
> Comment on attachment 791594 [details] [diff] [review]
> bug-898712.patch
> 
> Review of attachment 791594 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Is there a way to add a test for this feature?

Yes. Kailas, please take a look at my tests for Bug 762593 to help you get started. Feel free to ask me questions if you have any.
(In reply to Ivan Alagenchev :ialagenchev from comment #21)
> (In reply to Camilo Viecco (:cviecco) from comment #20)
> > Comment on attachment 791594 [details] [diff] [review]
> > bug-898712.patch
> > 
> > Review of attachment 791594 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Is there a way to add a test for this feature?
> 
> Yes. Kailas, please take a look at my tests for Bug 762593 to help you get
> started. Feel free to ask me questions if you have any.
I looked at the test cases for Bug 762593 and understood how to write test cases. However, it is not clear to me how to send SSL certificates with various errors (such as expired certificate, wrong certificate, etc). In particular,  Is there any way in mochitest to establish a https site and send different types of certificate?
Comment on attachment 791594 [details] [diff] [review]
bug-898712.patch

For the /devtools/webconsole changes, you need review from someone on the devtools team (like msucan).

For the /security/manager/ssl/src changes, Brian could do the review.
Attachment #791594 - Flags: review?(tanvi) → review+
Comment on attachment 791594 [details] [diff] [review]
bug-898712.patch

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

::: security/manager/ssl/src/SSLServerCertVerification.cpp
@@ +221,5 @@
> +      nsCOMPtr<nsIScriptError> scriptError;
> +      scriptError = do_CreateInstance(NS_SCRIPTERROR_CONTRACTID);
> +      if (scriptError) {
> +        nsresult rv;
> +        rv = scriptError->Init(message,

nit: nsresult rv = scriptError->Init(...), or simplify the code like this:

    if (console && NS_SUCCEEDED(scriptError->Init(...))

nit Please change the text wrapping so that we fit as much text on the line up to 80 chars as possible. i.e. don't put each new argument on its own line.

@@ +237,1 @@
>        console->LogStringMessage(message.get());

Please remove the console->LogStringMessage() call. If there is an error creating scriptError or there is an error initializing it, it is fine to just not log the message in those error cases. That's what other code is doing. Then, you can also remove the early return.

r+ with the above changes.

It seems like a similar changes is needed in HandshakCallback (in nsNSSCallbacks.cpp) and nsHandleSSLError (in nsNSSIOLayer.cpp). If/when you make those changes, please factor the common boilerplate logic into a separate function. These changes in this second paragraph can happen either in a follow-up patch in this bug, or in a new bug.

I will defer to others about whether we require an automated test for this.
Attachment #791594 - Flags: review?(brian) → review+
Posted patch bug-898712.patch (obsolete) — Splinter Review
Patch generated on hg tip: changeset:   143295:d136c8999d96

Updated patch incorporated brian's suggestions. 

I tested this patch manually using security report tool extension I am developing on https://csptest.computerist.org website. 

Now I will try to create automated test cases.
Attachment #791594 - Attachment is obsolete: true
Attachment #793402 - Flags: review?(brian)
Comment on attachment 793402 [details] [diff] [review]
bug-898712.patch

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

Good work, but like I said in the previous review, we should avoid the code duplication. I show you how below.

Please speak up if you run into trouble with the test. I wouldn't be surprised if you were to run into trouble that would make writing a test disproportionately difficult in relation to the value of having an automatic test.

::: security/manager/ssl/src/SSLServerCertVerification.cpp
@@ +228,1 @@
>      }

Please factor this logic out into a function, like this:

void
LogSimpleConsoleError(const nsAString & message,
                      const char * classification)
  nsCOMPtr<nsIScriptError> scriptError = 
    do_CreateInstance(NS_SCRIPTERROR_CONTRACTID);
  if (scriptError) {
    nsCOMPtr<nsIConsoleService> console = 
      do_GetService(NS_CONSOLESERVICE_CONTRACTID);
    if (console && NS_SUCCEEDED(scriptError->Init(message, EmptyString(),
		EmptyString(), 0, 0,
		nsIScriptError::errorFlag,
		classification))) {
      console->LogMessage(scriptError);
    }
  }
}


Then you should be able to re-use this function to make all the logging calls one-liners:

LogSimpleConsoleError(message, "SSL");

IMO, such a function would be best put in nsContentUtils, but if you want to stuff it in PSM for now you could put it in nsNSSIOLayer.h (declaration) / nsNSSIOLayer.cpp (definition).
Attachment #793402 - Flags: review?(brian) → review-
Posted patch bug-898712.patch (obsolete) — Splinter Review
Thanks! brain for your feedback! It helped me to improve the patch code.
Attachment #793402 - Attachment is obsolete: true
Attachment #793956 - Flags: review?(brian)
Posted patch bug-898712.patch (obsolete) — Splinter Review
Sorry, previous patch had some nits (extra blank space).
Attachment #793956 - Attachment is obsolete: true
Attachment #793956 - Flags: review?(brian)
Attachment #793958 - Flags: review?(brian)
Comment on attachment 793958 [details] [diff] [review]
bug-898712.patch

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

Drive by feedback!

There is a lot of overlap with this bug (and also with Bug 890224) and with the Security Console Utils work in Bug 897240. This is partially because the work in 897240 is designed to help your use case. Is it possible for you to reuse some of that work to achieve your goals here? A big difference seems to be that you want to report the errors generated by the underlying code directly, while the Security Console Utils work concerns itself with localization (a prerequisite for reporting anything to the Developer Tools, and indeed, ideally, anywhere).

::: browser/devtools/webconsole/webconsole.js
@@ +4549,5 @@
>        case "Mixed Content Blocker":
>        case "CSP":
>        case "Invalid HSTS Headers":
>        case "Insecure Password Field":
> +      case "SSL":

There is no reason to include this. Since you always init your nsIScriptError's with a innerWindowID of 0 in LogSimpleConsoleError, they will never make it any window's Web Console (they don't know which Web Console to be delivered to).

If you goal here was to report these messages to the Web Console, you're going to have to rethink some things. (They will still appear in the Browser Console no matter what).
(In reply to Garrett Robinson [:grobinson] from comment #31)
> Comment on attachment 793958 [details] [diff] [review]
> bug-898712.patch
> 
> Review of attachment 793958 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Drive by feedback!
> 
> There is a lot of overlap with this bug (and also with Bug 890224) and with
> the Security Console Utils work in Bug 897240. This is partially because the
> work in 897240 is designed to help your use case. Is it possible for you to
> reuse some of that work to achieve your goals here? A big difference seems
> to be that you want to report the errors generated by the underlying code
> directly, while the Security Console Utils work concerns itself with
> localization (a prerequisite for reporting anything to the Developer Tools,
> and indeed, ideally, anywhere).
> 
> ::: browser/devtools/webconsole/webconsole.js
> @@ +4549,5 @@
> >        case "Mixed Content Blocker":
> >        case "CSP":
> >        case "Invalid HSTS Headers":
> >        case "Insecure Password Field":
> > +      case "SSL":
> 
> There is no reason to include this. Since you always init your
> nsIScriptError's with a innerWindowID of 0 in LogSimpleConsoleError, they
> will never make it any window's Web Console (they don't know which Web
> Console to be delivered to).
> 
According to comment #10, I included it to associate SSL errors with Security Category instead of JS category. 

> If you goal here was to report these messages to the Web Console, you're
> going to have to rethink some things. (They will still appear in the Browser
> Console no matter what).
(In reply to Garrett Robinson [:grobinson] from comment #31)
> There is a lot of overlap with this bug (and also with Bug 890224) and with
> the Security Console Utils work in Bug 897240. This is partially because the
> work in 897240 is designed to help your use case. Is it possible for you to
> reuse some of that work to achieve your goals here? A big difference seems
> to be that you want to report the errors generated by the underlying code
> directly, while the Security Console Utils work concerns itself with
> localization (a prerequisite for reporting anything to the Developer Tools,
> and indeed, ideally, anywhere).

I suggest we just land Kailas patch as-is, and then we file another bug for the localization and/or conversion to the utils class.
> ::: browser/devtools/webconsole/webconsole.js
> @@ +4549,5 @@
> >        case "Mixed Content Blocker":
> >        case "CSP":
> >        case "Invalid HSTS Headers":
> >        case "Insecure Password Field":
> > +      case "SSL":
> 
> There is no reason to include this. Since you always init your
> nsIScriptError's with a innerWindowID of 0 in LogSimpleConsoleError, they
> will never make it any window's Web Console (they don't know which Web
> Console to be delivered to).

Browser console doesn't care about the classification of messages?

> If you goal here was to report these messages to the Web Console, you're
> going to have to rethink some things. (They will still appear in the Browser
> Console no matter what).

Also worth filing a bug for.
(In reply to Brian Smith (:briansmith, was :bsmith@mozilla.com) from comment #33)
> > ::: browser/devtools/webconsole/webconsole.js
> > @@ +4549,5 @@
> > >        case "Mixed Content Blocker":
> > >        case "CSP":
> > >        case "Invalid HSTS Headers":
> > >        case "Insecure Password Field":
> > > +      case "SSL":
> > 
> > There is no reason to include this. Since you always init your
> > nsIScriptError's with a innerWindowID of 0 in LogSimpleConsoleError, they
> > will never make it any window's Web Console (they don't know which Web
> > Console to be delivered to).
> 
> Browser console doesn't care about the classification of messages?
> 

My mistake - I was thinking of the old Error Console, and forgot that the new Browser Console uses the same filtering code as the Web Console. I retract my earlier comment.
Comment on attachment 793958 [details] [diff] [review]
bug-898712.patch

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

grobinson, do you see any problem with us checking this in now, given the discussion above?
Attachment #793958 - Flags: review?(brian)
Attachment #793958 - Flags: review+
Attachment #793958 - Flags: feedback?(grobinson)
Comment on attachment 793958 [details] [diff] [review]
bug-898712.patch

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

Nope! The logging situation is kind of all over the place now. Bug 897240 was intended to be a neat solution to this as well as other logging needs, but I don't think we should block Kaila's work on it. I say check it in!
Attachment #793958 - Flags: feedback?(grobinson) → feedback+
This has feedback+ and review+. Are we OK with checking it in?
(In reply to Frederik Braun [:freddyb] from comment #37)
> This has feedback+ and review+. Are we OK with checking it in?

I believe we are all in consensus on this.
Keywords: checkin-needed
Kailas, can you rechange the patch so it meets the criteria from this document (e.g. you need to specify bug number, describe the patch and say who reviewed it)? https://developer.mozilla.org/en/Creating_a_patch_that_can_be_checked_in

After that, please re-add the checkin-needed keyword :)
Flags: needinfo?(patilkr24)
Keywords: checkin-needed
Posted patch Bug-898712.patch (obsolete) — Splinter Review
Addresed comment #39: rechange the patch so it meets the criteria.
Attachment #793958 - Attachment is obsolete: true
Attachment #810548 - Flags: checkin+
Flags: needinfo?(patilkr24)
Keywords: checkin-needed
Attachment #810548 - Flags: review+
Attachment #810548 - Flags: checkin+
Thanks for looking again. I think this still needs the title (commit message)...
Example: hg qrefresh -m "Bug 12345 - Test bug, r=grobinson"
It's explained on the MDN link in comment #39.
Freddyb: I did exactly the same, but I realize my mistake. After hg qrefresh -m " ...." I used command "hg qdiff > bug-898712.patch" command to generate a patch. Instead I should have copied patch from ".hg/patches" folder.
Posted patch Bug-898712Splinter Review
patch ready for checked in.
Attachment #810548 - Attachment is obsolete: true
Attachment #810608 - Flags: review+
(In reply to Kailas from comment #42)
> Freddyb: I did exactly the same, but I realize my mistake. After hg qrefresh
> -m " ...." I used command "hg qdiff > bug-898712.patch" command to generate
> a patch. Instead I should have copied patch from ".hg/patches" folder.

For future reference, `hg export qtip > somebug.patch` will do the right thing with the topmost applied patch in your queue.
https://hg.mozilla.org/integration/fx-team/rev/f57dd2d6ec4d

Thanks for the patch! In the future, can you please include your name in your patches as well?
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/f57dd2d6ec4d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.