Closed Bug 367698 Opened 13 years ago Closed 10 years ago

avoid pre-populating "(no subject)" text in subject box.

Categories

(Thunderbird :: Message Compose Window, enhancement)

enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0b3

People

(Reporter: BijuMailList, Assigned: clarkbw)

References

Details

Attachments

(3 files, 4 obsolete files)

Attached image no_subject.png
Thunderbird
version 3 alpha 1 (20070121)

see first attachment...

It is irritating to see a mail without subject line.
More irritating to see something with "(no subject)".

Please make subject field mandatory on outgoing mail.
for incoming mail allow user(reader) to provide one.

Current implementation allow to send mail with "(no subject)".
also user can accidentally send with subject "(no subject)Hi".
bug 119684 is about disabling the warning, but nobody yet proposed to make the subject mandatory
Assignee: mscott → nobody
Severity: normal → enhancement
I think it would be best to avoid a new UI for entering in the subject as well as avoid offering a default subject text.

A short dialog that warns against sending without a subject should get the message to people.  The dialog should just cancel sending or send them message without a subject.

+-------------------------------------------------+
|  Send this message without a subject?           |
|                                                 |
|     [ Send without Subject ] [ Cancel Sending ] |
+-------------------------------------------------+

It might make sense to also offer to not warn about this again as bug 119684 seems to cover.

+-------------------------------------------------+
|  Send this message without a subject?           |
|                                                 |
|  [x] Always warn about empty subjects           |
|                                                 |
|     [ Send without Subject ] [ Cancel Sending ] |
+-------------------------------------------------+

This addition of "[x] Always warn..." comes with a cost of adding a preference to the composition tab such that people can undo the pref change and get warnings back.  I'm not really excited about a preference like that, which makes me slightly hesitant to ask for this.

I can understand there is a desire to force people to write subjects, but we can't practically do that.  Lack of subjects in emails may be irritating, but even if we forced everyone using Thunderbird to use subjects we couldn't force everyone to use Thunderbird.  It could make a lot of sense to look at handling a lack of subject better in the UI display. 
This change removes the '(no subject)' entry prompt in favour of a confirm dialogue as described in comment #2.  I used the confirmEx function so I could also fix bug 119684 at the same time.  This means you can un-check the 'Always check for empty subjects' checkbox and it would no longer prompt you at all.

I'm not adding a pref into the preferences for undoing the "always check" checkbox.  The only way to reset this is in the config editor options and I think that's going to be ok for a while, if not indefinitely.-

Phil, how does this look?  ( I made this on windows so I hope the formatting isn't all messed up )
Attachment #377373 - Flags: review?(philringnalda)
I think bug 119684 should be wontfix. The bastards who insist on sending messages with no subject deserve to be annoyed.

Also, although verbs on the buttons is usually a good thing, in the proposal it's just repetition of the text. Maybe it should say something like "This message doesn't have a subject yet." ?
(In reply to comment #4)
> I think bug 119684 should be wontfix. The bastards who insist on sending
> messages with no subject deserve to be annoyed.

I partially agree, however I'm sympathizing with people who are using email as a command tool and don't need subjects for that usage.  I especially don't like creating it because then people will want a way to bring the dialog back.  Dunno...

> Also, although verbs on the buttons is usually a good thing, in the proposal
> it's just repetition of the text. Maybe it should say something like "This
> message doesn't have a subject yet." ?

Good point, I'll use a version of your sentence in the next patch.
Ok, I didn't have to think about this very long to see that you're right.  The stop nagging me thing is only really useful when TB is forcing you to add some kind of subject.  With a prompt that you forgot a subject I think that's enough.

And for bonus points I added a Subject.focus() call on cancel!
Assignee: nobody → clarkbw
Attachment #377373 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #377734 - Flags: review?(philringnalda)
Attachment #377373 - Flags: review?(philringnalda)
Cancel should be the default action, no? And why the capital S in "... Subject?"
No I'm making send w/o a subject the default.  I want to be helpful and remind people in case they forgot to add a subject but not punish for not adding a subject on purpose.  Enter will continue to send and Cancel will allow you to enter a subject.

Good catch on the capital S, one more patch coming.  Do you want to do the final review for me?
Attached patch updated fix for strings (obsolete) — Splinter Review
fix for the capital S string catch.

Also wanted to note that in my first patch I had removed the "(no subject)" string from the properties file but found that it was still needed by a function that sets the window title so I brought it back.  Just figured I should explain that one since it's not all that obvious.

setting to magnus for review
Attachment #377734 - Attachment is obsolete: true
Attachment #377764 - Flags: review?(mkmelin+mozilla)
Attachment #377734 - Flags: review?(philringnalda)
setting the right bugzilla knobs
Assignee: clarkbw → nobody
Component: General → Message Compose Window
OS: Windows XP → All
QA Contact: general → message-compose
Hardware: x86 → All
Target Milestone: --- → Thunderbird 3.0b3
Comment on attachment 377764 [details] [diff] [review]
updated fix for strings

Some thoughts
 o having a question in the dialog title is a bit unusual, maybe use something like "Subject Missing" or "Subject Reminder"
   * either way, i think title words should be capitalized
 o there's no access keys for the buttons (you add that by putting "&" before some char, like "&Send Without...."

>diff --git a/mail/components/compose/content/MsgComposeCommands.js b/mail/components/compose/content/MsgComposeCommands.js
>--- a/mail/components/compose/content/MsgComposeCommands.js
>+++ b/mail/components/compose/content/MsgComposeCommands.js
>@@ -1660,21 +1660,19 @@
>           if (gPromptService)
>           {

While you're in here, I this it's safe to assume gPromptService is set...

>             var bundle = document.getElementById("bundle_composeMsgs");
>-            var result = {value: bundle.getString("defaultSubject")};
>-            if (gPromptService.prompt(
>+            if (gPromptService.confirmEx(
>                     window,
>-                    bundle.getString("sendMsgTitle"),
>-                    bundle.getString("subjectDlogMessage"),
>-                    result,
>-                    null,
>-                    {value:0}))
>+                    bundle.getString("subjectEmptyTitle"),

... and please drop this to 2 space indention instead of 4.
(In reply to comment #12)
> (From update of attachment 377764 [details] [diff] [review])
> Some thoughts
>  o having a question in the dialog title is a bit unusual, maybe use something
> like "Subject Missing" or "Subject Reminder"
>    * either way, i think title words should be capitalized

That would make it more consistent, using "Subject Reminder"

>  o there's no access keys for the buttons (you add that by putting "&" before
> some char, like "&Send Without...."

&Send Without Subject
&Cancel Sending
> 
> >diff --git a/mail/components/compose/content/MsgComposeCommands.js b/mail/components/compose/content/MsgComposeCommands.js
> >--- a/mail/components/compose/content/MsgComposeCommands.js
> >+++ b/mail/components/compose/content/MsgComposeCommands.js
> >@@ -1660,21 +1660,19 @@
> >           if (gPromptService)
> >           {
> 
> While you're in here, I this it's safe to assume gPromptService is set...

removed

> 
> >             var bundle = document.getElementById("bundle_composeMsgs");
> >-            var result = {value: bundle.getString("defaultSubject")};
> >-            if (gPromptService.prompt(
> >+            if (gPromptService.confirmEx(
> >                     window,
> >-                    bundle.getString("sendMsgTitle"),
> >-                    bundle.getString("subjectDlogMessage"),
> >-                    result,
> >-                    null,
> >-                    {value:0}))
> >+                    bundle.getString("subjectEmptyTitle"),
> 
> ... and please drop this to 2 space indention instead of 4.

done, though the indentation doesn't look quite right to me now but I think this is how you want it.

I also changed the mode line at the top since it was switching me to 4 spaces when we actually want 2.  Hope that's ok.
Attachment #377764 - Attachment is obsolete: true
Attachment #378654 - Flags: review?(mkmelin+mozilla)
Attachment #377764 - Flags: review?(mkmelin+mozilla)
Comment on attachment 378654 [details] [diff] [review]
updated strings and cleaned up tabbing

Looks good!

>+          if (gPromptService.confirmEx(
>+            window,
>+            bundle.getString("subjectEmptyTitle"),
>+            bundle.getString("subjectEmptyMessage"),

I meant 2 spaces from where a row would have started, like

          if (gPromptService.confirmEx(
                window,
                bundle.getString("subjectEmptyTitle"),
                bundle.getString("subjectEmptyMessage"),

r=mkmelin with that.
Attachment #378654 - Flags: review?(mkmelin+mozilla) → review+
updated with the correct tabbing.

carrying forward the r=mkmelin
Attachment #378654 - Attachment is obsolete: true
Attachment #378931 - Flags: review+
thanks magnus!

adding checkin-needed keyword
Keywords: checkin-needed
Assignee: nobody → clarkbw
http://hg.mozilla.org/comm-central/rev/7b28a6f2c5af
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Depends on: 497572
You need to log in before you can comment on or make changes to this bug.