Closed
Bug 1098371
Opened 10 years ago
Closed 9 years ago
Create localized version of sslv3 error page
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
People
(Reporter: Gijs, Assigned: Gijs)
References
Details
Attachments
(2 files, 1 obsolete file)
1.88 KB,
patch
|
mconley
:
review+
Gijs
:
checkin+
|
Details | Diff | Splinter Review |
12.96 KB,
patch
|
mconley
:
review+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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)
Comment 1•10 years ago
|
||
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)
Comment 2•10 years ago
|
||
Note that bug 1084025 will produce much more ssl_error_no_cypher_overlap other than SSLv3 only server. We will really need bug 1084986...
Comment 3•10 years ago
|
||
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.
Assignee | ||
Comment 4•9 years ago
|
||
(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)
Assignee | ||
Comment 5•9 years ago
|
||
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 | ||
Updated•9 years ago
|
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(gijskruitbosch+bugs)
Updated•9 years ago
|
Iteration: --- → 37.1
Flags: qe-verify?
Flags: firefox-backlog+
Comment 6•9 years ago
|
||
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+
Assignee | ||
Comment 7•9 years ago
|
||
Noticed that I (a) missed two strings, and (b) I added the l10n notes.
Assignee | ||
Updated•9 years ago
|
Attachment #8531776 -
Attachment is obsolete: true
Attachment #8531776 -
Flags: review?(gavin.sharp)
Assignee | ||
Updated•9 years ago
|
Attachment #8532565 -
Flags: review?(gavin.sharp)
Attachment #8532565 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
Attachment #8532565 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Comment 8•9 years ago
|
||
remote: https://hg.mozilla.org/integration/fx-team/rev/816e1db8855f remote: https://hg.mozilla.org/releases/mozilla-aurora/rev/7e4ef557e13b
Assignee | ||
Updated•9 years ago
|
Keywords: leave-open
Comment 9•9 years ago
|
||
Hi Gijs, can you provide a point value.
Flags: needinfo?(gijskruitbosch+bugs)
Assignee | ||
Updated•9 years ago
|
Points: --- → 5
Flags: qe-verify?
Flags: qe-verify+
Flags: needinfo?(gijskruitbosch+bugs)
Comment 10•9 years ago
|
||
I hate myself for not noticing this before, but are we supposed to use an hard-coded "Firefox" in the string?
Assignee | ||
Comment 11•9 years ago
|
||
(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)
Assignee | ||
Comment 13•9 years ago
|
||
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)
Updated•9 years ago
|
status-firefox36:
--- → affected
status-firefox37:
--- → fixed
Comment 14•9 years ago
|
||
Gijs, not sure to understand, you requested an aurora uplift but already landed the change?!
Flags: needinfo?(gijskruitbosch+bugs)
Assignee | ||
Comment 16•9 years ago
|
||
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?
Assignee | ||
Comment 17•9 years ago
|
||
(resetting the flags because while the strings are there, we still need to implement using them...)
status-firefox36:
affected → ---
status-firefox37:
fixed → ---
Updated•9 years ago
|
Iteration: 37.1 → 37.2
Assignee | ||
Updated•9 years ago
|
Attachment #8532565 -
Flags: checkin+
Assignee | ||
Updated•9 years ago
|
No longer depends on: 1107731
Summary: Update error page for sslv3 - nightly edition → Create localized version of sslv3 error page
Assignee | ||
Comment 19•9 years ago
|
||
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)
Updated•9 years ago
|
Iteration: 37.2 → 37.3
Comment 20•9 years ago
|
||
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 21•9 years ago
|
||
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+
Assignee | ||
Comment 22•9 years ago
|
||
(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)
Comment 23•9 years ago
|
||
(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)
Assignee | ||
Comment 24•9 years ago
|
||
remote: https://hg.mozilla.org/integration/fx-team/rev/ffce954c598e
Assignee | ||
Updated•9 years ago
|
Keywords: leave-open
Comment 25•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ffce954c598e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Assignee | ||
Comment 26•9 years ago
|
||
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?
Assignee | ||
Comment 27•9 years ago
|
||
[Tracking Requested - why for this release]: Currently no improved error is in place for 36.
Assignee | ||
Comment 28•9 years ago
|
||
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]
Comment 29•9 years ago
|
||
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)
Assignee | ||
Comment 30•9 years ago
|
||
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)
Comment 31•9 years ago
|
||
(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)
Comment 32•9 years ago
|
||
(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.
Updated•9 years ago
|
Attachment #8539477 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 33•9 years ago
|
||
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]
Updated•9 years ago
|
Comment 34•9 years ago
|
||
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)
Assignee | ||
Comment 35•9 years ago
|
||
(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)
Comment 36•9 years ago
|
||
(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)
Assignee | ||
Comment 37•9 years ago
|
||
(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/ .
Comment 38•9 years ago
|
||
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.
Assignee | ||
Comment 39•9 years ago
|
||
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.
Description
•