458788 - The "Preserve threading when sorting messages" UI shows wrong state

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: General, defect)

Description

[Stanimir Stamenkov]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.13) Gecko/20080313 SeaMonkey/1.1.9 (Spidey; Mnenhy 0.7.5.0)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20081005 SeaMonkey/2.0a2pre

The UI for the Mail & Newsgroups preference "Preserve threading when sorting messages" appears to be "inverted", that is it takes effect when unchecked and doesn't take effect when checked.

According to Neil's reply in mozilla.dev.apps.seamonkey:

The bug is in pref-mailnews.xul - "reversed" on the checkbox should be "inverted" on the preference element.

Reproducible: Always

Comment 1

[Stefan [:stefanh]]

Apparently it worked with the old pref system

Comment 2

[[e-mail]]

Sure, the old pref system didn't have separate preference elements so there was no confusion over where the attribute belonged.

Comment 3

[Stefan [:stefanh]]

Attachment: fix

Tested this by looking at the value changes in about:config.

Comment 4

[Jens Hatlak (:InvisibleSmiley)]

Attachment: What Neil said

Let's see if I got this right.

Comment 5

[Stefan [:stefanh]]

Comment on attachment 341969 [details] [diff] [review]
What Neil said

Looks good to me ;)

Comment 6

[[e-mail]]

Comment on attachment 341969 [details] [diff] [review]
What Neil said

Although a trawl through other pref panels suggests that our style seems to be to put the inverted and type attributes on their own line or lines.

Comment 7

[Stefan [:stefanh]]

Jens, the best thing to do in order to get this landed is to add "checkin-needed" in the Keywords field.

Comment 8

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #6)
> (From update of attachment 341969 [details] [diff] [review])
> Although a trawl through other pref panels suggests that our style seems to be
> to put the inverted and type attributes on their own line or lines.

The choice is yours, Stefan's patch has the "inverted" attribute on its own line (and he was faster) but we can come up with yet another version. :-)

(In reply to comment #7)
> Jens, the best thing to do in order to get this landed is to add
> "checkin-needed" in the Keywords field.

Sure. I would also have requested review in the first place if I had tested the patch (which you did). That's what you get for watching TV while creating patches. ;-)

Comment 9

[Serge Gautherie (:sgautherie)]

(In reply to comment #6)

Neil, I too would prefer Stefan's patch...

Jens, did your prepare your patch "manually" ?
It's not a Hg patch, and the two files don't even have the same "level" of directories :-/

Comment 10

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #9)
> Jens, did your prepare your patch "manually" ?

Using MXR and diff, yes. For me it was faster that way.

> It's not a Hg patch, and the two files don't even have the same "level" of
> directories :-/

I verified that the downloaded version of the file (from MXR/comm-central) is equal to the one from hg (my local checkout), though but I understand that having the full path has certain advantages. I'll use hg diff (with the settings from the Mercurial FAQ at MDC) in the future.

Comment 11

[Stefan [:stefanh]]

Jens patch should apply if one just removes the "patch" in
+++ patch/pref-mailnews.xul"

Comment 12

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #11)
> Jens patch should apply if one just removes the "patch" in
> +++ patch/pref-mailnews.xul"

Actually it will apply without modification by simply using 'patch < 458788.patch'. The +++ line is completely irrelevant as far as patch is concerned.

Anyway, I'm sorry for the trouble I've caused. I'd supply a corrected patch but actually you already provided that in the first place. I'd say let's just commit yours, I'm fine with that. I don't know how to proceed, though. Remove the checkin-needed keyword, assign the bug to you, obsolete my patch? How can the reviews be transferred? I guess I don't have the rights to do that...

Comment 13

[Justin Wood (:Callek)]

Jens, if this hasn't been committed once I arrive home tonight (a few hours from now) I'll commit with Neil's nits addressed, crediting you; as that seems to be what stefanh prefers anyway :-)

Thanks!

Comment 14

[Justin Wood (:Callek)]

$ hg push
pushing to ssh://hg.mozilla.org/comm-central/
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 1 changes to 1 files

$ hg tip -v   
changeset:   558:1d810e40645f
tag:         tip
user:        Jens Hatlak <[e-mail]>
date:        Wed Oct 08 20:30:43 2008 -0400
files:       mailnews/base/prefs/resources/content/pref-mailnews.xul
description:
Bug 458788, The "Preserve threading when sorting messages" UI shows wrong state
r+sr=NeilAway