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

RESOLVED FIXED in seamonkey2.0a2

Status

SeaMonkey
MailNews: General
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: Stanimir Stamenkov, Assigned: InvisibleSmiley)

Tracking

Trunk
seamonkey2.0a2
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

2.06 KB, patch
neil@parkwaycc.co.uk
: review+
neil@parkwaycc.co.uk
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

10 years ago
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

10 years ago
Apparently it worked with the old pref system
Blocks: 404263
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 2

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

Comment 3

10 years ago
Created attachment 341968 [details] [diff] [review]
fix

Tested this by looking at the value changes in about:config.
Assignee: nobody → stefanh
Status: NEW → ASSIGNED
Attachment #341968 - Flags: superreview?(neil)
Attachment #341968 - Flags: review?(neil)
(Assignee)

Comment 4

10 years ago
Created attachment 341969 [details] [diff] [review]
What Neil said

Let's see if I got this right.

Updated

10 years ago
Attachment #341968 - Flags: superreview?(neil)
Attachment #341968 - Flags: review?(neil)

Comment 5

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

Looks good to me ;)
Attachment #341969 - Flags: superreview?(neil)
Attachment #341969 - Flags: review?(neil)

Comment 6

10 years ago
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.
Attachment #341969 - Flags: superreview?(neil)
Attachment #341969 - Flags: superreview+
Attachment #341969 - Flags: review?(neil)
Attachment #341969 - Flags: review+

Updated

10 years ago
Assignee: stefanh → jh

Comment 7

10 years ago
Jens, the best thing to do in order to get this landed is to add "checkin-needed" in the Keywords field.
(Assignee)

Comment 8

10 years ago
(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. ;-)
Keywords: checkin-needed
(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 :-/
Flags: in-testsuite-
Version: unspecified → Trunk
(Assignee)

Comment 10

10 years ago
(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.

Updated

10 years ago
Attachment #341968 - Attachment is obsolete: true

Comment 11

10 years ago
Jens patch should apply if one just removes the "patch" in
+++ patch/pref-mailnews.xul"
(Assignee)

Comment 12

10 years ago
(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...
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!
$ 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 <jh@junetz.de>
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
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Keywords: checkin-needed
OS: Windows XP → All
Hardware: PC → All
Target Milestone: --- → seamonkey2.0a2
You need to log in before you can comment on or make changes to this bug.