Closed Bug 995737 Opened 11 years ago Closed 10 years ago

Adapt seamonkey for the addressbook remote content policy change; use the permission manager instead of address book property

Categories

(SeaMonkey :: MailNews: Address Book & Contacts, defect)

defect
Not set
normal

Tracking

(seamonkey2.35 affected, seamonkey2.36 affected, seamonkey2.37 affected, seamonkey2.38 fixed)

RESOLVED FIXED
seamonkey2.38
Tracking Status
seamonkey2.35 --- affected
seamonkey2.36 --- affected
seamonkey2.37 --- affected
seamonkey2.38 --- fixed

People

(Reporter: mkmelin, Assigned: philip.chee)

References

(Blocks 2 open bugs, )

Details

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #457296 +++ When bug 457296 lands AllowRemoteContent property in the address book will be gone. For the remote content in mail buttons to work, SeaMonkey should adapt the code to use the permission manager instead. The Thunderbird UI is in bug 953426 (which is still awaiting review), with some small changes in bug 457296. At least you want to call something like allowRemoteContentForURI, and remove the "allow remote" checkboxes from ab. See especially attachment 8405845 [details] [diff] [review].
No longer blocks: 457296
Depends on: 457296
Version: unspecified → Trunk
Depends on: 953426
Summary: adapt seamonkey for the ab remote content policy change; use permission manager instead of address book property → Adapt seamonkey for the addressbook remote content policy change; use the permission manager instead of address book property
> # LOCALIZATION NOTE (remoteContentBarMessage): %S is the brandname > remoteContentBarMessage=To protect your privacy, %S has blocked remote content in this message. > -remoteContentBarButton=Show Remote Content > -remoteContentBarButtonKey=S > +remoteContentPrefLabel=Options > +remoteContentPrefAccesskey=O Thunderbird uses Options/Preferences. This doesn't sound quite right. How about "Actions" instead? Differences with the Thunderbird implementation: 1. The list of blockable sites/hosts in Thunderbird is held in an attribute on the XUL popup element as a space separated strings. I am using a property on gMessageNotificationBar. remoteHosts is a Set(). 2. TB spends a lot of time converting the hosts back and forth between strings and nsIURIs. In my Set, the keys are always strings and I only convert to a URI at the point of adding the host to the permissions manager.
Assignee: nobody → philip.chee
Status: NEW → ASSIGNED
Attachment #8602247 - Flags: review?(iann_bugzilla)
Comment on attachment 8602247 [details] [diff] [review] Patch v1.0 Proposed implementation. Should we add an "Exceptions" button next to the preferences for "Block images and other content from remote sources"? It will be interesting find an access key though! Need to raise a bug about updating help pages too.
Flags: needinfo?(philip.chee)
Attachment #8602247 - Flags: review?(iann_bugzilla) → review+
Comment on attachment 8602247 [details] [diff] [review] Patch v1.0 Proposed implementation. >+ remoteHosts: null, >+ >+ setRemoteContentMsg: function(aMsgHdr, aContentURI) >+ { >+ // remoteHosts is a Set of all blockable hosts. >+ if (!this.remoteHosts) >+ this.remoteHosts = new Set(); When you "reset" this set, you set it to a new Set. So why not initialise it to a new Set too, that way you know it's always a Set. (Or, reset it to null instead, so it's only a Set if there's something in the set.) >+ hosts.splice(0, 0, authorEmailAddress); unshift >+ if(!Services.prefs.getBoolPref("browser.preferences.instantApply")) >+ ReloadMessage(); ??? (Also, space after if.)
(In reply to Ian Neal from comment #3) > Comment on attachment 8602247 [details] [diff] [review] > Patch v1.0 Proposed implementation. > > Should we add an "Exceptions" button next to the preferences for "Block > images and other content from remote sources"? It will be interesting find > an access key though! Our UI is a wee bit more crowded than TB. Do we have space for that?
Flags: needinfo?(philip.chee)
Attachment #8602247 - Flags: superreview?(mnyromyr)
Comment on attachment 8602247 [details] [diff] [review] Patch v1.0 Proposed implementation. moa+ by inspection. Just some nits: >+function onRemoteContentOptionsShowing(aEvent) { Move { onto new line. >+ for (let child of childNodes) { >+ child.remove(); >+ } No braces needed here. >+ for (let host of hosts) { >+function allowRemoteContentForURI(aItem) { >+function editRemoteContentSettings() { Move { onto new line.
Attachment #8602247 - Flags: superreview?(mnyromyr) → superreview+
Target Milestone: --- → seamonkey2.38
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment on attachment 8602247 [details] [diff] [review] Patch v1.0 Proposed implementation. >+ var headerParser = MailServices.headerParser; >+ // update the allow remote content for sender string >+ var mailbox = headerParser.extractHeaderAddressMailboxes(aMsgHdr.author); >+ var emailAddress = mailbox || aMsgHdr.author; >+ var displayName = headerParser.extractHeaderAddressName(aMsgHdr.author); Unused? >+ MailServices.headerParser.parseHeadersWithArray( >+ gMessageDisplay.displayedMessage.author, addresses, {}, {}); >+ var authorEmailAddress = addresses.value[0]; Nit: parseHeadersWithArray is deprecated, use parseEncodedHeader()[0].email instead. >+ if(!Services.prefs.getBoolPref("browser.preferences.instantApply")) >+ ReloadMessage(); Still ???
Re browser.preferences.instantApply, it's to reload the message to reflect changes you might have done. See bug 926473 comment 13 or so.
So... this says it is fixed in 2.38 but I'm on 2.40 and I can't make remote content show by selecting Options --> "Allow remote content for <email_address>" Am I missing something?
I had been using 2.38 because it did seem to be fixed there, but 2.39 was broken again to the point where I went back to 2.38. I migrated to 2.40 recently, and see that it is still broken. When given the option list on a message, clicking on one of the <email_address> entries results in varied results like the clicked on result may or may not disappear from the list and that may or may not result in a change in what is displayed in the email message. The only thing that does seem to work is the first entry in the list (show remote content in this message), but it does not get remembered in subsequent Seamonkey sessions. You have to go through the options list again. I will go back to the 2.38 version again, when I am sufficiently annoyed.
Blocks: 763284
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: