Comment 276 for bug 52667

Revision history for this message
In , Blake Winton (bwinton) wrote :

(In reply to comment #220)
> >+ <xul:menuseparator anonid="hdrReplyAllSubButtonSep" />
> Drop the space before />

Done.

> >+ let numAddresses = hdrParser.parseHeadersWithArray(uniqueRecipients, {}, {},
> >+ {});
> The 80 char limit isn't quite this strict, i think. Or move more than one of
> the {}s to the next line too.

Done.

> >+ let showReplyList = currentHeaderData['list-post'];
> Prefer to use " instead of ' when it's not needed to use '.
> Here and later on.

Done and done.

> >+ // But, if we're in a news folder, we should default to the Reply button.
> >+ if (isNewsURI(msgHdr.folder.URI))
> Should probably to the same for feeds (check msgHdr.folder.type == "rss")

Yeah, that doesn't seem to work.

I tested it, and there was no "type" attribute on msgHdr.folder, and when I looked through http://mxr.mozilla.org/comm-central/source/mailnews/base/public/nsIMsgFolder.idl#71 I didn't see anything that looked useful.

I hoped I could base my calculations off of the URI, but for my RSS feed, it's:
mailbox://nobody@Feeds/Blog-o%21
whereas for my Local Folders, it's
mailbox://nobody@Local%20Folders/Inbox/Admin
So that doesn't seem like a good thing to use.

If there's a way to differentiate RSS feeds from Local Folders, I would love to handle this case in the same way I handle the News case.

> >+ // If it's a news message, show the ReplyAll sub-button and separator.
> >+ // otherwise, hide them.
> Comment placement isn't consistent here. Move the "// If it's..." inside the
> if clause

Done.

> r=mkmelin with that

Thank you! There's a new patch with the changes I could make coming soon.

Later,
Blake.