Deal with removal of tooltip code from PSM

RESOLVED FIXED in seamonkey2.21

Status

SeaMonkey
Security
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: neil@parkwaycc.co.uk, Assigned: neil@parkwaycc.co.uk)

Tracking

Trunk
seamonkey2.21

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Bug #832848 wants to remove nsISecureBrowserUI's tooltipText attribute.
(Assignee)

Comment 1

5 years ago
Created attachment 747950 [details] [diff] [review]
Proposed patch
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #747950 - Flags: review?(philip.chee)
Comment on attachment 747950 [details] [diff] [review]
Proposed patch

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

I filed bug 872761 to make Firefox's logic for this more re-usable, but I'm not sure if it will be appropriate for SeaMonkey.

::: suite/browser/nsBrowserStatusHandler.js
@@ +371,5 @@
> +                               .SSLStatus.serverCert;
> +        var issuerName = cert.issuerOrganization ||
> +                         cert.issuerCommonName || cert.issuerName;
> +        if (issuerName == "RSA Data Security, Inc.")
> +          issuerName = "Verisign, Inc.";

I would avoid doing this special name handling. The Firefox code doesn't do it, for example.

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#6517
Neil, thanks for jumping on this. My understanding is that SeaMonkey bugs like this shouldn't block Platform bugs except in exceptional circumstances, so I'm replacing the blocking relationship with a "See also" relationship. In particular, I would like to make this change soon because there are other Platform bug fixes that depend on this Platform change.
No longer blocks: 832848
See Also: → bug 832848
(Assignee)

Comment 4

5 years ago
(In reply to Brian Smith from comment #2)
> (From update of attachment 747950 [details] [diff] [review])
> > +        if (issuerName == "RSA Data Security, Inc.")
> > +          issuerName = "Verisign, Inc.";
> 
> I would avoid doing this special name handling. The Firefox code doesn't do
> it, for example.
> 
> http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#6517

Actually I was copying from here:

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/pageinfo/security.js#96

Comment 5

5 years ago
> I would avoid doing this special name handling. The Firefox code doesn't do it,
> for example.

You removed the Verisign code in your patch:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c74ce1905b96#l11.57
But you never explained why. Bug 47435 might be not necessary now but I don't know. Could you ask the security team?

q.v. Bug 47435 Substitute "Verisign" for "RSA Data Security"
Flags: needinfo?(bsmith)
(In reply to Philip Chee from comment #5)
> > I would avoid doing this special name handling. The Firefox code doesn't do it,
> > for example.
> 
> You removed the Verisign code in your patch:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/c74ce1905b96#l11.57
> But you never explained why. Bug 47435 might be not necessary now but I
> don't know. Could you ask the security team?
> 
> q.v. Bug 47435 Substitute "Verisign" for "RSA Data Security"

I filed bug 872910 to fix the Page Info dialog box.

Either you can be consistent with Firefox's site identity block UI or you can be consistent with the page info UI. Ideally, we'd get a patch for bug 872910 and then remove the name hack from the SeaMonkey site identity UI. But, it's up to you guys as to whether to do that now in this bug or in a separate bug.

I CC'd kwilson, who is probably the only one that could find out whether there is any expectation from VeriSign as far as this naming hack goes.
Flags: needinfo?(bsmith)
(Assignee)

Comment 7

5 years ago
(In reply to Philip Chee from comment #5)
> You removed the Verisign code in your patch:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/c74ce1905b96#l11.57
He removed all of the code that generates the tooltipText attribute, not just the RSA/Verisign check...
(In reply to Kathleen Wilson from bug 872910 comment #1)
> From a representative of Symantec/VeriSign: "I agree that you can remove
> that code. The RSA root expired in 2005 and if it's still in your trust
> store you can remove it."
> 
> Also, I checked my spreadsheet of included root certs, and this root is no
> longer included, and we don't currently have expired root certs included in
> NSS.
(Assignee)

Comment 9

5 years ago
Created attachment 750629 [details] [diff] [review]
Removed RSA hack
Attachment #747950 - Attachment is obsolete: true
Attachment #747950 - Flags: review?(philip.chee)
Attachment #750629 - Flags: review?(philip.chee)

Comment 10

5 years ago
Comment on attachment 750629 [details] [diff] [review]
Removed RSA hack

> Removed RSA hack
Could you file a followup bug to remove the RSA hack from suite/browser/pageinfo/security.js ? Thanks!

> -#nsContextMenu.js (setWallpaper confirmation dialog)
> -wallpaperConfirmTitle  = Set As Wallpaper
> -wallpaperConfirmButton = Set Wallpaper
> -wallpaperConfirmMsg    = Do you want to set this image as your desktop wallpaper?
Good catch!
Attachment #750629 - Flags: review?(philip.chee) → review+
(Assignee)

Comment 11

5 years ago
Pushed comm-central changeset 9b76619bdfe4.

Rather conveniently the m-c change got backed out and didn't make Gecko 23, thus we don't need a late-l10 aurora landing.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Updated

5 years ago
Target Milestone: --- → seamonkey2.21
You need to log in before you can comment on or make changes to this bug.