Closed Bug 1098371 Opened 10 years ago Closed 9 years ago

Create localized version of sslv3 error page

Categories

(Firefox :: General, defect)

defect
Not set
normal
Points:
5

Tracking

()

VERIFIED FIXED
Firefox 37
Iteration:
37.3 - 12 Jan
Tracking Status
firefox36 + verified
firefox37 --- verified
firefox38 --- verified

People

(Reporter: Gijs, Assigned: Gijs)

References

Details

Attachments

(2 files, 1 obsolete file)

Like bug 1096695, but this time we should use decent localizable strings. Might also have a go at getting more of the styling updated, although I'm confused because we /just/ did that...

Michael, can you elaborate as to why we need to change font sizes and/or colors again after the updates from bug 982347 (landed for Firefox 32)? Is the change intentional?
Flags: needinfo?(mmaslaney)
Gijs, bug 982347 was completed when there was a less defined style. We are continuing to iterate and are much closure than we were when bug 982347.
Flags: needinfo?(mmaslaney)
Blocks: 1096695
Note that bug 1084025 will produce much more ssl_error_no_cypher_overlap other than SSLv3 only server.
We will really need bug 1084986...
I agree, we need to address the issue of ssl_error_no_cypher_overlap not being specific enough for the front-end to properly distinguish various cases.
(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #3)
> I agree, we need to address the issue of ssl_error_no_cypher_overlap not
> being specific enough for the front-end to properly distinguish various
> cases.

Unfortunately, we now also missed the 36 string cut-off. I'd like to land these strings as late-l10n on aurora so we get translations for 36, at least. We can do the actual error page work when bug 1084986 is fixed. Needinfo'ing myself so I don't forget (again).
Flags: needinfo?(gijskruitbosch+bugs)
IMO we should land these on 36. Gavin/Francesco, can you please have a look?
Attachment #8531776 - Flags: review?(gavin.sharp)
Attachment #8531776 - Flags: feedback?(francesco.lodolo)
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Flags: needinfo?(gijskruitbosch+bugs)
Iteration: --- → 37.1
Flags: qe-verify?
Flags: firefox-backlog+
Comment on attachment 8531776 [details] [diff] [review]
strings for improved sslv3 error,

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

::: browser/locales/en-US/chrome/overrides/netError.dtd
@@ +205,5 @@
>  <!ENTITY remoteXUL.title "Remote XUL">
>  <!ENTITY remoteXUL.longDesc "<p><ul><li>Please contact the website owners to inform them of this problem.</li></ul></p>">
> +
> +<!ENTITY oldSecurityProtocol.title "Unable to Connect Securely">
> +<!ENTITY oldSecurityProtocol.longDesc "Firefox cannot guarantee the safety of your data on <span id='hostname'></span> because it uses SSLv3, a broken security protocol.">

Just to be safe, can you add a comment explaining that localizers shouldn't touch the <span> part, especially the id?

I wonder if we could inject the protocol name as we do for the URL, that would save us from the next bug with XYZ protocol.
Attachment #8531776 - Flags: feedback?(francesco.lodolo) → feedback+
Depends on: 1107731
No longer depends on: 1084986
Noticed that I (a) missed two strings, and (b) I added the l10n notes.
Attachment #8531776 - Attachment is obsolete: true
Attachment #8531776 - Flags: review?(gavin.sharp)
Attachment #8532565 - Flags: review?(gavin.sharp)
Attachment #8532565 - Flags: approval-mozilla-aurora?
Attachment #8532565 - Flags: review?(gavin.sharp) → review+
Keywords: leave-open
Hi Gijs, can you provide a point value.
Flags: needinfo?(gijskruitbosch+bugs)
Points: --- → 5
Flags: qe-verify?
Flags: qe-verify+
Flags: needinfo?(gijskruitbosch+bugs)
I hate myself for not noticing this before, but are we supposed to use an hard-coded "Firefox" in the string?
(In reply to Francesco Lodolo [:flod] from comment #10)
> I hate myself for not noticing this before, but are we supposed to use an
> hard-coded "Firefox" in the string?

No. Good point. :-(
Flags: needinfo?(gijskruitbosch+bugs)
Updated:

remote:   https://hg.mozilla.org/integration/fx-team/rev/6ed7044ca837

remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/195cd832724a


with new ids because rather safe than sorry.
Flags: needinfo?(gijskruitbosch+bugs)
Gijs, not sure to understand, you requested an aurora uplift but already landed the change?!
Flags: needinfo?(gijskruitbosch+bugs)
Comment on attachment 8532565 [details] [diff] [review]
strings for improved sslv3 error, f=flod,r+ l10n=gavin/flod

This is one of those rare cases where real life (particularly, talking to folks that were in Portland) was quicker than bugzilla, and so bugzilla state didn't reeeeaaaaally reflect what was going on. I got what I interpreted as a+ from gavin, so I landed stuff, and I forgot to update the bug accordingly. Sorry for the confusion!
Flags: needinfo?(gijskruitbosch+bugs)
Attachment #8532565 - Flags: approval-mozilla-aurora?
(resetting the flags because while the strings are there, we still need to implement using them...)
Iteration: 37.1 → 37.2
Attachment #8532565 - Flags: checkin+
Blocks: 1113780
No longer depends on: 1107731
Summary: Update error page for sslv3 - nightly edition → Create localized version of sslv3 error page
Here's at least a patch to use the strings on 37/36 while we don't have the new error yet.
Attachment #8539477 - Flags: review?(gavin.sharp)
Iteration: 37.2 → 37.3
Comment on attachment 8539477 [details] [diff] [review]
create localized version of sslv3 error page,

Sorry that I haven't been able to get to this - looks straightforward, but hoping mconley can step in to look more closely.
Attachment #8539477 - Flags: review?(gavin.sharp) → review?(mconley)
Comment on attachment 8539477 [details] [diff] [review]
create localized version of sslv3 error page,

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

Looks good. Thanks Gijs!

::: browser/base/content/aboutNetError.xhtml
@@ +155,5 @@
>            // change id to the replaced child's id so styling works
>            errDesc.id = "errorLongDesc";
>          }
>  
> +        if (err == "nssFailure2" && 

Nit: trailing ws

@@ +350,5 @@
>  
> +      function learnMoreSSLV3() {
> +        try {
> +          location.href = "https://support.mozilla.org/kb/how-resolve-sslv3-error-messages-firefox";
> +        } catch (ex) {/* Avoid breaking in case this throws weird errors */}

Can we log whatever exception happens here? If a user clicks on this, and this does happen to error out for some reason, and the button is made to be disabled... it'd be nice if we had some way of getting the exception from the console.
Attachment #8539477 - Flags: review?(mconley) → review+
(In reply to Mike Conley (:mconley) - Needinfo me! from comment #21)
> Comment on attachment 8539477 [details] [diff] [review]
> create localized version of sslv3 error page,
> 
> Review of attachment 8539477 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good. Thanks Gijs!
> 
> ::: browser/base/content/aboutNetError.xhtml
> @@ +155,5 @@
> >            // change id to the replaced child's id so styling works
> >            errDesc.id = "errorLongDesc";
> >          }
> >  
> > +        if (err == "nssFailure2" && 
> 
> Nit: trailing ws
> 
> @@ +350,5 @@
> >  
> > +      function learnMoreSSLV3() {
> > +        try {
> > +          location.href = "https://support.mozilla.org/kb/how-resolve-sslv3-error-messages-firefox";
> > +        } catch (ex) {/* Avoid breaking in case this throws weird errors */}
> 
> Can we log whatever exception happens here? If a user clicks on this, and
> this does happen to error out for some reason, and the button is made to be
> disabled... it'd be nice if we had some way of getting the exception from
> the console.

We could, but I'm just copying what the other bits of this page already do... except that I just realized they specify the potential for error (namely reloading a URL that throws an exception).

I don't really expect this to throw, so maybe we should just remove the try catch. Does that sound OK?
Flags: needinfo?(mconley)
(In reply to :Gijs Kruitbosch from comment #22)
> I don't really expect this to throw, so maybe we should just remove the try
> catch. Does that sound OK?

It does indeed. Thanks!
Flags: needinfo?(mconley)
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/ffce954c598e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Comment on attachment 8539477 [details] [diff] [review]
create localized version of sslv3 error page,

Approval Request Comment
[Feature/regressing bug #]: n/a
[User impact if declined]: confusing no_cypher_overlap error page for sslv3 sites
[Describe test coverage new/current, TreeHerder]: no :-(
[Risks and why]: pretty low, mostly just altering some error page content in the case of sslv3 errors
[String/UUID change made/needed]: no, but this uses strings that were landed on 36.
Attachment #8539477 - Flags: approval-mozilla-beta?
[Tracking Requested - why for this release]:
Currently no improved error is in place for 36.
Per discussion with gavin on IRC, we think that:
- if we get the improved error code NSS code landed on beta, we should take the improved error wording on beta for the new error code.
- otherwise, we should not.

See bug 1123967 and bug 1109859 for people who are bitten by the error message being very specific despite the error code being relatively generic.

This might mean this will be wontfix for 36.
Whiteboard: [do NOT land on beta unless/until the fix from bug 1084986 gets uplifted with an NSS fix via bug 1107731]
I verified the fix for this issue on Nightly 38.0a1 (2015-01-25) and Aurora 37.0a2 (2015-01-25) using a few l10n builds across all platforms. The correct neterror details are displayed on both branches but there are a few locales that are either not localized (e.g. fa, he) or just partially localized (e.g. de, ar).

Gijs, I'm not sure which of the locales were targeted by this fix - please take a look at this etherpad [1] which contains detailed test results.

[1] https://etherpad.mozilla.org/Bug1098371
Flags: needinfo?(gijskruitbosch+bugs)
This wasn't a late-string landing or something like this, so we'll follow the "usual" pattern for translating these strings, AIUI. Any missing/half-translated things should be corrected by the relevant l10n teams. Francesco, is that right?
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(francesco.lodolo)
(In reply to :Gijs Kruitbosch from comment #30)
> This wasn't a late-string landing or something like this, so we'll follow
> the "usual" pattern for translating these strings, AIUI. Any
> missing/half-translated things should be corrected by the relevant l10n
> teams. Francesco, is that right?

Exactly. Of those 4 locales only German is always up-to-date, but they work on Aurora, so translation will be completed before the end of the cycle.

The only 'safe' locale for this kind of testing on central is 'it'. For aurora cs, es-ES, pl, ru, sk are good picks as well.

The "missing" column in this view also helps
https://l10n.mozilla.org/shipping/dashboard?tree=fx_aurora
Flags: needinfo?(francesco.lodolo)
(In reply to Francesco Lodolo [:flod] from comment #31)
> (In reply to :Gijs Kruitbosch from comment #30)
> > This wasn't a late-string landing or something like this, so we'll follow
> > the "usual" pattern for translating these strings, AIUI. Any
> > missing/half-translated things should be corrected by the relevant l10n
> > teams. Francesco, is that right?
> 
> Exactly. Of those 4 locales only German is always up-to-date, but they work
> on Aurora, so translation will be completed before the end of the cycle.
> 
> The only 'safe' locale for this kind of testing on central is 'it'. For
> aurora cs, es-ES, pl, ru, sk are good picks as well.
> 
> The "missing" column in this view also helps
> https://l10n.mozilla.org/shipping/dashboard?tree=fx_aurora

Gijs, Francesco - thank you. I believe it's safe to mark this fix as verified.
Status: RESOLVED → VERIFIED
Attachment #8539477 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
https://hg.mozilla.org/releases/mozilla-beta/rev/e6cefc687439
Whiteboard: [do NOT land on beta unless/until the fix from bug 1084986 gets uplifted with an NSS fix via bug 1107731]
Depends on: 1127339
I verified the fix for this issue on Windows 7 64bit, Windows 8 32bit, Ubuntu 13.10 32bit and Mac OSX 10.9.5 using Firefox 36 Beta 8, buildID: 20150209164123 (builds: cs, es-ES, pl, ru, sk, it). One mention should be done here: the error code displayed on the page is "ssl_error_no_cypher_overlap" - it is the correct one? (on latest Nightly and latest Aurora, the error code is "ssl_error_unsupported_version").
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to Camelia Badau, QA [:cbadau] from comment #34)
> I verified the fix for this issue on Windows 7 64bit, Windows 8 32bit,
> Ubuntu 13.10 32bit and Mac OSX 10.9.5 using Firefox 36 Beta 8, buildID:
> 20150209164123 (builds: cs, es-ES, pl, ru, sk, it). One mention should be
> done here: the error code displayed on the page is
> "ssl_error_no_cypher_overlap" - it is the correct one? (on latest Nightly
> and latest Aurora, the error code is "ssl_error_unsupported_version").

This doesn't make sense to me. Even on en-US, for beta, I'm seeing the wrong error page and the wrong error code.

It looks like maybe the NSS changes are not fully there? Which also doesn't make sense because bug 1107731 and bug 1113780 were both uplifted. Kai/Masatoshi, do you know what's going on here?
Flags: needinfo?(kaie)
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(VYV03354)
(In reply to :Gijs Kruitbosch from comment #35)
> (In reply to Camelia Badau, QA [:cbadau] from comment #34)
> > I verified the fix for this issue on Windows 7 64bit, Windows 8 32bit,
> > Ubuntu 13.10 32bit and Mac OSX 10.9.5 using Firefox 36 Beta 8, buildID:
> > 20150209164123 (builds: cs, es-ES, pl, ru, sk, it). One mention should be
> > done here: the error code displayed on the page is
> > "ssl_error_no_cypher_overlap" - it is the correct one? (on latest Nightly
> > and latest Aurora, the error code is "ssl_error_unsupported_version").
> 
> This doesn't make sense to me. Even on en-US, for beta, I'm seeing the wrong
> error page and the wrong error code.
> 
> It looks like maybe the NSS changes are not fully there? Which also doesn't
> make sense because bug 1107731 and bug 1113780 were both uplifted.
> Kai/Masatoshi, do you know what's going on here?

I saw ssl_error_unsupported_version and red icon on <https://contact.russlavbank.com/> (from bug 1107014) with the latest Firefox 36 beta. How did you verified?
Flags: needinfo?(VYV03354)
(In reply to Masatoshi Kimura [:emk] from comment #36)
> (In reply to :Gijs Kruitbosch from comment #35)
> > (In reply to Camelia Badau, QA [:cbadau] from comment #34)
> > > I verified the fix for this issue on Windows 7 64bit, Windows 8 32bit,
> > > Ubuntu 13.10 32bit and Mac OSX 10.9.5 using Firefox 36 Beta 8, buildID:
> > > 20150209164123 (builds: cs, es-ES, pl, ru, sk, it). One mention should be
> > > done here: the error code displayed on the page is
> > > "ssl_error_no_cypher_overlap" - it is the correct one? (on latest Nightly
> > > and latest Aurora, the error code is "ssl_error_unsupported_version").
> > 
> > This doesn't make sense to me. Even on en-US, for beta, I'm seeing the wrong
> > error page and the wrong error code.
> > 
> > It looks like maybe the NSS changes are not fully there? Which also doesn't
> > make sense because bug 1107731 and bug 1113780 were both uplifted.
> > Kai/Masatoshi, do you know what's going on here?
> 
> I saw ssl_error_unsupported_version and red icon on
> <https://contact.russlavbank.com/> (from bug 1107014) with the latest
> Firefox 36 beta. How did you verified?

Latest 36 beta on OS X, using https://sslv3.dshield.org/ .
Hm, when I set "security.tls.version.fallback-limit" to 3, Firefox 36 displayed the expected error page on <https://sslv3.dshield.org/>.
Looks like sslv3.dshield.org is tolerant at TLS 1.2 (that is, try to negotiate SSLv3 "correctly" then we fail the handshake), but intolerant at TLS 1.0 (that is, they fail the handshake and expect we will fallback with SSLv3). Unfortunately we cannot detect the latter case reliably because the intolerance reason is not limited to version mismatch.
OK, sounds like this is working as expected on beta, then. Thanks!
Flags: needinfo?(kaie)
You need to log in before you can comment on or make changes to this bug.