15.06 KB, image/png
787 bytes, patch
|Details | Diff | Splinter Review|
1.40 KB, patch
|Details | Diff | Splinter Review|
Currently the junk mail notification bar consists of three items: 1. An icon 2. A small snippet of text saying "Junk Mail" 3. A button saying "Not Junk" The text saying Junk mail is not informative enough. Maybe having it saying something like "Thunderbird thinks this is junk mail" instead. I bet there is an ux-principle for this.
It feels awfully close to ux-jargon or ux-tone. Making it a proper sentence would make it sound more natural.
Hi. This is my first attempt at contributing. How about "This message is marked as junk mail" ?
Attachment #618324 - Flags: feedback+
Comment on attachment 618324 [details] Screenshot of new junk mail notification I think Blake Winton is an appropriate person to request feedback from, for this change.
Attachment #618324 - Flags: feedback+ → feedback?(bwinton)
Comment on attachment 618324 [details] Screenshot of new junk mail notification I think I prefer "Thunderbird thinks this message is junk mail.", which matches "Firefox is having trouble recovering your windows and tabs.", and "Firefox has prevented this site from opening a window." (Also, please end the sentence with a period.) This will make your patch a little more complicated, since you'll need to use the "brandShortName" entity, but hopefully there is some code you can steal from that does just that. ;) So, I'm going to say, f+ with those fixed. Thanks, Blake.
Attachment #618324 - Flags: feedback?(bwinton) → feedback+
Hm, maybe this would be a good place to have tri-state junk info? (See bug 366491.) And possibly likelihood it is junk. (Can be a follow-up of course.)
Assignee: nobody → heliosbryan
Hardware: x86_64 → All
Ran xpcshell and check tests. Check runs fine, xpcshell has failures, but there are the same number of failures before the patch and after the patch (5).
Attachment #618909 - Flags: review+
Hi Magnus - as I am new to development, not sure if I can take that additional portion on (assuming you were directing that to me).
Bryan, thats ok, it can be done in another followup-bug.
Comment on attachment 618909 [details] [diff] [review] Patch for bug 590226 Review of attachment 618909 [details] [diff] [review]: ----------------------------------------------------------------- To ask for review you should set the review flag to ? and choose a reviewer. ::: mail/locales/en-US/chrome/messenger/messenger.dtd @@ -774,4 @@ > <!ENTITY contextUnmute.accesskey "m"> > > <!-- Junk Bar --> > -<!ENTITY junkBarMessage1.label "Junk Mail"> Unfortunately, when we change strings we also need to change the key (junkBarMessage1.label needs to change to something else), otherwise localizers can't notice changes. Yes, it's a pain :(
Attachment #618909 - Flags: review+ → review-
I'm not exactly sure I get how the localization mechanism works, or why it's junkBarMessage1.label and not just junkBarMessage.label. Would the appropriate change be to make it junkBarMessage2.label? Also, is there any other place in the source that I need to worry about changing for this?
yes, for instance change it to junkBarMessage2.label. You have to change it there and then of course change it to your new name where it's used.
Attachment #619148 - Flags: review?(mkmelin+mozilla)
Comment on attachment 619148 [details] [diff] [review] Patch for bug 590226 - v2 Thanks for the patch! r=mkmelin
Attachment #619148 - Flags: review?(mkmelin+mozilla) → review+
Adding checkin-needed keyword. Magnus or Blake, would you be willing to push this patch for Bryan?
Whiteboard: [good first bug] → [good first bug][checkin needed for comm-central]
Committed to comm-central as http://hg.mozilla.org/comm-central/rev/4209ab9786ef Thanks Bryan!
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 15.0
7 years ago
Whiteboard: [good first bug][checkin needed for comm-central] → [good first bug]
You need to log in before you can comment on or make changes to this bug.