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

RESOLVED FIXED in seamonkey2.38

Status

defect
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: mkmelin, Assigned: philip.chee)

Tracking

(Blocks 1 bug)

Trunk
seamonkey2.38
Dependency tree / graph

SeaMonkey Tracking Flags

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

Details

()

Attachments

(1 attachment)

Reporter

Description

5 years ago
+++ 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
Assignee

Updated

4 years ago
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
Assignee

Updated

4 years ago
Duplicate of this bug: 1148507
Assignee

Comment 2

4 years ago
>  # 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 3

4 years ago
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.)
Assignee

Comment 5

4 years ago
(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)
Assignee

Updated

4 years ago
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+
Assignee

Comment 7

4 years ago
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/3c00ed4749ee
Target Milestone: --- → seamonkey2.38
Assignee

Updated

4 years ago
Assignee

Updated

4 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 4 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 ???
Reporter

Comment 9

4 years ago
Re browser.preferences.instantApply, it's to reload the message to reflect changes you might have done. See bug 926473 comment 13 or so.

Comment 10

3 years ago
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?

Comment 11

3 years ago
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.
See Also: → 1223741
You need to log in before you can comment on or make changes to this bug.