Bug 7207 - Only set priority if it is different from normal
: Only set priority if it is different from normal
Status: RESOLVED FIXED
Product: Email
General
: 5.0/(2.2009.51-1)
: All Maemo
: Low enhancement (vote)
: ---
Assigned To: unassigned
: modest-bugs
:
: patch
:
:
  Show dependency tree
 
Reported: 2009-12-21 23:29 UTC by Adam Sjøgen
Modified: 2010-01-22 12:13 UTC (History)
2 users (show)

See Also:


Attachments
Only set priority if not normal (663 bytes, patch)
2009-12-21 23:29 UTC, Adam Sjøgen
Details
Updated patch for 3.1.18+0m5 (1.05 KB, patch)
2010-01-14 17:17 UTC, Adam Sjøgen
Details
Try using enum value instead of hardcoding 0 (1.08 KB, patch)
2010-01-21 02:14 UTC, Adam Sjøgen
Details


Note

You need to log in before you can comment on or make changes to this bug.


Description Adam Sjøgen (reporter) 2009-12-21 23:29:05 UTC
SOFTWARE VERSION:
(Settings > General > About product)
1.2009.42-11

EXACT STEPS LEADING TO PROBLEM: 
(Explain in detail what you do (e.g. tap on OK) and what you see (e.g. message
Connection Failed appears))
1. Send an email with normal priority
2. Headers X-MSMail-Priority: Normal and X-Priority: 3 are included in the
email sent

EXPECTED OUTCOME:
No extra headers for normal email.

ACTUAL OUTCOME:
 X-MSMail-Priority: Normal
 X-Priority: 3 

REPRODUCIBILITY:
(always, less than 1/10, 5/10, 9/10)
always

EXTRA SOFTWARE INSTALLED:

OTHER COMMENTS:
I think it makes sense to consider to only set the priority, hence sending the
extra headers, if the priority differs from normal.

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.6)
Gecko/20091216 Iceweasel/3.5.6 (like Firefox/3.5.6; Debian-3.5.6-1)
Comment 1 Adam Sjøgen (reporter) 2009-12-21 23:29:41 UTC
Created an attachment (id=1822) [details]
Only set priority if not normal

Here is a patch implementing the proposed change.
Comment 2 Andre Klapper maemo.org 2009-12-22 16:02:35 UTC
Thanks for the patch!

Hmm, why do you consider the current behaviour a bug?
Comment 3 Adam Sjøgen (reporter) 2009-12-22 16:22:36 UTC
(In reply to comment #2)

> Hmm, why do you consider the current behaviour a bug?

I meant to classify it as an 'enchancement', sorry.

I think it is a nice improvement not to set superfluous headers.

In comparison: Outlook only sets priority headers if you change importance to
high or low (i.e. away from default), Thunderbird works the same way (but has 5
levels), and Gmail does not seem to have the concept of priority. (I haven't
had time to test other email clients).
Comment 4 Adam Sjøgen (reporter) 2010-01-14 17:17:42 UTC
Created an attachment (id=1974) [details]
Updated patch for 3.1.18+0m5
Comment 5 Andre Klapper maemo.org 2010-01-19 16:51:31 UTC
Sergio, can you take a look at the patch here?
Comment 6 Sergio Villar Senin 2010-01-19 20:32:46 UTC
(In reply to comment #4)
> Created an attachment (id=1974) [details] [details]
> Updated patch for 3.1.18+0m5
> 

First of all thx for the patch. Secondly the patch works but I'd like to have a
different way to check the presence of the normal priority flag. Since it's a
flag (defined in tny-enums.h) it's better to use the enumerated values for that
instead of relying in the fact that currently the normal priority is
represented by 0.
Comment 7 Adam Sjøgen (reporter) 2010-01-21 02:14:08 UTC
Created an attachment (id=2073) [details]
Try using enum value instead of hardcoding 0

Thanks for the feedback.

Attached is a new patch with the 0 switched out for
TNY_HEADER_FLAG_NORMAL_PRIORITY.

I have only compiled it, I haven't actually tried running it.
Comment 8 Adam Sjøgen (reporter) 2010-01-21 02:17:28 UTC
(In reply to comment #7)
> Created an attachment (id=2073) [details] [details]
> Try using enum value instead of hardcoding 0

I just realized that I am probably comparing too much; I hope some of you who
do bit-fiddling in C on a daily basis will step up and change the comparison to
be correct.

Thanks.
Comment 9 Sergio Villar Senin 2010-01-21 14:22:52 UTC
(In reply to comment #7)
> Created an attachment (id=2073) [details] [details]
> Try using enum value instead of hardcoding 0
> 
> Thanks for the feedback.
> 
> Attached is a new patch with the 0 switched out for
> TNY_HEADER_FLAG_NORMAL_PRIORITY.
> 
> I have only compiled it, I haven't actually tried running it.

Do it next time :). I have to add a missing initialization when setting the
flags, because otherwise the "!=" comparison is not valid.

Fixed in
modest-3-2: 03ed09a05cde5f6bed1b2cfdbc342b1257d77b3a
master: 0afa1b62e5b7e9f56227bc5071ee8fcf4a67cb2f

Thank you very much for the patch and welcome on board, it's always great to
have new commits from community.
Comment 10 Adam Sjøgen (reporter) 2010-01-21 22:31:26 UTC
(In reply to comment #9)

>> Attached is a new patch with the 0 switched out for
>> TNY_HEADER_FLAG_NORMAL_PRIORITY.

>> I have only compiled it, I haven't actually tried running it.

> Do it next time :). I have to add a missing initialization when setting the
> flags, because otherwise the "!=" comparison is not valid.

Uhm, I can't see how the two things are related? Whether I check against 0 or
TNY_HEADER_FLAG_NORMAL_PRIORITY doesn't change anything with regards to
initialization?

The first two patches I have been using since I created them, so since December
21st, 2009 and January 14th, 2010.

The one with 0 switched out for TNY_HEADER_FLAG_NORMAL_PRIORITY is the only one
I haven't actually run with (yet).

Why does the initialization need to change because I compare against
0/TNY_HEADER_FLAG_NORMAL_PRIORITY?

And how did you figure out that your change to the initialization was needed?

> Fixed in
> modest-3-2: 03ed09a05cde5f6bed1b2cfdbc342b1257d77b3a
> master: 0afa1b62e5b7e9f56227bc5071ee8fcf4a67cb2f

> Thank you very much for the patch and welcome on board, it's always great to
> have new commits from community.

Thanks - I really enjoy being able to fix little niggles in the applications.

I am a little confused though: How come my name is on those two commits, when
they include a change I a) haven't contributed, and b) don't understand?

(I don't mean to sound harsh, but it seems a little wrong to include your
change under my name?)
Comment 11 Sergio Villar Senin 2010-01-22 11:06:17 UTC
(In reply to comment #10)
> (In reply to comment #9)
> 
> >> Attached is a new patch with the 0 switched out for
> >> TNY_HEADER_FLAG_NORMAL_PRIORITY.
> 
> >> I have only compiled it, I haven't actually tried running it.
> 
> > Do it next time :). I have to add a missing initialization when setting the
> > flags, because otherwise the "!=" comparison is not valid.
> 
> Uhm, I can't see how the two things are related? Whether I check against 0 or
> TNY_HEADER_FLAG_NORMAL_PRIORITY doesn't change anything with regards to
> initialization?
> 
> The first two patches I have been using since I created them, so since December
> 21st, 2009 and January 14th, 2010.
> 
> The one with 0 switched out for TNY_HEADER_FLAG_NORMAL_PRIORITY is the only one
> I haven't actually run with (yet).
> 
> Why does the initialization need to change because I compare against
> 0/TNY_HEADER_FLAG_NORMAL_PRIORITY?
> 
> And how did you figure out that your change to the initialization was needed?

Well as you say it's not really needed. I was just a bit picky with it, it's
just I prefer to have it like this to make it more visible that they're just
flags.

> > Fixed in
> > modest-3-2: 03ed09a05cde5f6bed1b2cfdbc342b1257d77b3a
> > master: 0afa1b62e5b7e9f56227bc5071ee8fcf4a67cb2f
> 
> > Thank you very much for the patch and welcome on board, it's always great to
> > have new commits from community.
> 
> Thanks - I really enjoy being able to fix little niggles in the applications.
> 
> I am a little confused though: How come my name is on those two commits, when
> they include a change I a) haven't contributed, and b) don't understand?
> 
> (I don't mean to sound harsh, but it seems a little wrong to include your
> change under my name?)

I just respected your name in the commit, I think credit belongs to you. I just
made a very small modification that does not really affect the patch very much.
I'm sorry if you feel bad with that, but I did it with my best intention.
Comment 12 Adam Sjøgen (reporter) 2010-01-22 12:13:33 UTC
(In reply to comment #11)

No worries.