319841 - Poor grammar in filter rules dialog: "For incoming messages that:"

Status: RESOLVED FIXED

(MailNews Core :: Filters, defect)

Description

[Lewis Rosenthal]

User-Agent:       Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.9a1) Gecko/20051209 MultiZilla/1.8.1.0u SeaMonkey/1.5a
Build Identifier: Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.9a1) Gecko/20051209 MultiZilla/1.8.1.0u SeaMonkey/1.5a

Poor choice of words in this instance. A better phrase would be "For incoming messages where:"

Reproducible: Always

Steps to Reproduce:
1. Select Tools | Message Filters...
2. Select a filter and click Edit or click New to create a new one


Actual Results:  
Dialog sheet shows poor grammar.

Expected Results:  
English grammar should be pithy and correct.

My mom is an English professor with several advanced degrees... I've had to listen to stuff like this since I was in grade school. Now, at 45, she still can badger me about things she picks up!

Comment 1

[Stephen Donner [:stephend] Not actively reading bugmail]

Not really, if you stop to think about this.

The dialog's two radio buttons are:

"Match all of the following" and "Match any of the following"

Now, if we implemented your/your mother's suggestion, our text would read as such: "For incoming messages where match all of the following" and "For incoming messages where match any of the following".

That doesn't sound right to me, though I'm certainly no linguist.

The text is designed to match those radio buttons; I think you're thinking they were intended to match the pulldowns, below, but that's not the case.

Comment 2

[Lewis Rosenthal]

(In reply to comment #1)
> Not really, if you stop to think about this.
> 
> The dialog's two radio buttons are:
> 
> "Match all of the following" and "Match any of the following"
> 
> Now, if we implemented your/your mother's suggestion, our text would read as
> such: "For incoming messages where match all of the following" and "For
> incoming messages where match any of the following".
> 
> That doesn't sound right to me, though I'm certainly no linguist.
> 
> The text is designed to match those radio buttons; I think you're thinking they
> were intended to match the pulldowns, below, but that's not the case.
> 
Fair enough, Stephen. Your point is well taken. In that case, a more proper word would be "which" instead of "that," e.g., "For incoming messages which match all of the following" and "For incoming messages which match any of the following."

Lewis

Comment 3

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #2)

<snip>

> Fair enough, Stephen. Your point is well taken. In that case, a more proper
> word would be "which" instead of "that," e.g., "For incoming messages which
> match all of the following" and "For incoming messages which match any of the
> following."
> 
> Lewis

Right.  Strunk and White's The Elements of Style, page 59, confirms that, since "which" is the nondefining, nonrestrictive use rather than "that," which would be defining or restrictive pronoun.

I'll make a patch which reflects this.  Thanks for the bug, and your patience.

Comment 4

[Lewis Rosenthal]

(In reply to comment #3)

<snip>

> I'll make a patch which reflects this.  Thanks for the bug, and your patience.
> 
Sounds good! I'm happy to help out where I can, even if it is for something small. ;-) Thanks for listening, and for preparing the patch!

Lewis

Comment 5

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Patch

Thunderbird trunk patch.

Comment 6

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Patch (Checked in trunk/branch 1.8 & 1.8.0)

SeaMonkey trunk patch

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 205560 [details] [diff] [review]
Patch (Checked in trunk/branch 1.8 & 1.8.0)

I can only review Address Book bugs passing to SeaMonkey mailnews peer.

Comment 8

[Karsten Düsterloh]

Comment on attachment 205560 [details] [diff] [review]
Patch (Checked in trunk/branch 1.8 & 1.8.0)

Fine be me, modulo the fact that I'm not a native English speaker.

Comment 9

[Stephen Donner [:stephend] Not actively reading bugmail]

Comment on attachment 205560 [details] [diff] [review]
Patch (Checked in trunk/branch 1.8 & 1.8.0)

Timeless landed this; it'll appear in tomorrow's 2005-12-13 SeaMonkey build.  Thunderbird's patch will land when mscott gets a chance to review it.

Comment 10

[Peter Weilbacher]

Comment on attachment 205560 [details] [diff] [review]
Patch (Checked in trunk/branch 1.8 & 1.8.0)

Stephen, just wondering: why did you obsolete this patch? If it was checked in should you not rather add [checked in] to its description?

Comment 11

[Giacomo Magnini]

(In reply to comment #10)
> (From update of attachment 205560 [details] [diff] [review] [edit])
> Stephen, just wondering: why did you obsolete this patch? If it was checked in
> should you not rather add [checked in] to its description?

I think this is timeless style for getting checked in patches out of the radar...
Do we want this for SM 1.0b?

Comment 12

[Robert Kaiser]

(In reply to comment #11)
> Do we want this for SM 1.0b?

a=me for SM 1.0b, still one other needed for checkin

Comment 13

[Ian Neal]

a=me too

Comment 14

[Mike Kaply [:mkaply]]

Not OS/2 specific

Comment 15

[Stephen Donner [:stephend] Not actively reading bugmail]

Comment on attachment 205560 [details] [diff] [review]
Patch (Checked in trunk/branch 1.8 & 1.8.0)

checked in

Comment 16

[Stefan [:stefanh]]

Comment on attachment 205560 [details] [diff] [review]
Patch (Checked in trunk/branch 1.8 & 1.8.0)

(In reply to comment #15)
> (From update of attachment 205560 [details] [diff] [review] [edit])
> checked in
> 

This doesn't seem to have been checked in on branch (for seamonkey 1.0)...

Comment 17

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #16)
> (From update of attachment 205560 [details] [diff] [review] [edit])
> (In reply to comment #15)
> > (From update of attachment 205560 [details] [diff] [review] [edit] [edit])
> > checked in
> > 
> 
> This doesn't seem to have been checked in on branch (for seamonkey 1.0)...

I was only responding to comment 10.  Someone else will have to land this on branch, I can't.

Comment 18

[Stefan [:stefanh]]

Comment on attachment 205560 [details] [diff] [review]
Patch (Checked in trunk/branch 1.8 & 1.8.0)

Mark, can you land this on branch, please?

Comment 19

[Mark Banner (:standard8)]

(In reply to comment #18)
> (From update of attachment 205560 [details] [diff] [review] [edit])
> Mark, can you land this on branch, please?

Checked in:
/cvsroot/mozilla/mailnews/base/search/resources/locale/en-US/FilterEditor.dtd,v  <--  FilterEditor.dtd
new revision: 1.32.2.1; previous revision: 1.32

Comment 20

[Stefan [:stefanh]]

SeaMonkey-only attachment #205560 [details] [diff] [review] checked in on branch by Mark.

Comment 21

[David :Bienvenu]

tb fix checked in.

Comment 22

[Ian Neal]

Comment on attachment 205560 [details] [diff] [review]
Patch (Checked in trunk/branch 1.8 & 1.8.0)

Checking in (branch 1.8.0)
FilterEditor.dtd;
new revision: 1.32.10.1; previous revision: 1.32
done