440368 - Tags not alphabetically sorted in if I rename the tag (Message>Tag-Menu/Tag-Button, Tools>Options>Display>Tags)

Status: RESOLVED WONTFIX

(Thunderbird :: Preferences, defect)

Description

[X. Em]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.1.14) Gecko/20080404 Firefox/2.0.0.14
Build Identifier: DE Thunderbird Version 2.0.0.14 (20080421)

new added keywords are alphabetically sorted in keyword list
renamed keyword remains at the same place, i.e. keyword list is no longer alphabetically sorted

Reproducible: Always

Steps to Reproduce:
1. add keyword (i.e. create new keyword) to mail
2. preferences/view/keywods shows the keywords alphabetically sorted
3. rename keyword in preferences/view/keywods

Actual Results:  
(new) keyword remain in the same place as the (old) keyword was

Expected Results:  
keywords always alphabetically sorted

Comment 1

[Steve Simms]

Assuming we're looking at the same thing, when I add a tag under Tools -> Options -> Display -> Tags, it's always added to the bottom of the list, and isn't sorted alphabetically.

English version, 2.0.0.16, WinXP.

Comment 2

[Thomas D. (:thomas8)]

TB Version 2.0.0.21 (20090302)

This bug should be confirmed. Sorting of tags, especially when renaming existing tags, is a mess which will make tags unusable for people with many tags. Some of the related behaviour is different depending from where you start out (and shouldn't be, of course)...

Assume the following existing tags:
Default tags:
1-5: Important, To do, etc. (these are never touched, as far as alphabetical sequence is concerned - why not?)
User-defined tags:
6: A-tag
7: C-tag

Scenarios:
A) Create a new tag (B1-tag) using Message > Tags > New tag...
--> Result: new tag is automatically added in the correct alphabetical order (both in the message-tag-list and in the settings-tag-list)
A-tag
B1-tag
C-tag

B1) Create a new tag (B2-tag) using Options > Settings > View > Tags
--> Result: new tag is *initially* added at the end of the settings-list:
A-tag
B1-tag
C-tag
B2-tag 
It will stay in the wrong order until you close and reopen the dialogue (no way to refresh). Not sure whether I'd prefer newly-created tags to be in alphabetical order immediately, but refresh should be possible!

B2) Close Settings dialogue, reopen...
--> and find everything sorted well:
A-tag
B1-tag
B2-tag
C-tag
(Likewise, Message > Tags - Menu shows them in the right order).

B3) From Settings dialogue, rename C-tag to AA-tag (as this bug reports: now we are getting into trouble...)
--> AA-tag (renamed from C-tag) will stay forever in the position of C-tag, so the final result is (for both lists):
A-tag
B1-tag
B2-tag
AA-tag
Very odd!! Do this a couple of times, and mess up your tags completely.

Extra oddity (which needs a separate bug):
Watch how your tags' keyboard shortcuts change (but only for Tags 6-9!!!) every time you are adding a new tag which happens to end up alphabetically at the beginning of your list. Again, this is very odd as keyboard shortcuts become unpredictable. We'll want to assign keyboard shortcuts to existing tags and not vice versa.

Just my 2 cents, but repairing this would be a waste of time.
For the benefit of everyone, please consider implementing/porting Firefox tagging system to Thunderbird (cf. Bug 370076 -  create tags for messages by typing with the keyboard). Firefox now even has tags autocomplete!

(More TB tag-related bugs: Bug 432710 - [Meta] Thunderbird Tag Bugs Tracker)

Comment 3

[Thomas D. (:thomas8)]

Confirming for current nightly of TB3 as well:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b4pre) Gecko/20090406 Shredder/3.0b3pre)

Some menu captions of my previous comment #2 are wrong because I translated them from the German version, sorry for that. Whereever you see
"Options > Settings > View > Tags" (Secenario B), what I really mean is
"Tools > Options > Display > Tags"

Comment 4

[Thomas D. (:thomas8)]

Ping! This bug is still present in current nightly, Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090606 Shredder/3.0b3pre

I'd appreciate confirmation of this major bug, which can render the supposedly alphabetical list of tags completely useless as the sorting will become chaos when you have lots of tags and rename (="Edit") some of them in a way that changes their initial letters. Please use the detailed steps to reproduce in comment #2.

Comment 5

[Wayne Mery (:wsmwk)]

how many tags are we talking about? 
I don't see this as major if less than 25-30 tags

Comment 6

[X. Em]

There are 259 tags at the moment; about one new tag a day. I am using the tags as 'project ids' (yyyy-nnnnm-cc_<misc text> with yyyy=year, nnnn=project number, m=indicator, cc=country code, <misc text>=project name).

Comment 7

[WADA:World Anti-bad-Duping Agency]

"tag of Tb" has three kinds of "name".
  mailnews.tags.tag-01.ordinal = "abc-a"  <- ordinal name(called Importance).
                                             string used for ordering.
                                             used by tag list of preference,
                                             sort of tag column of thread pane  
                                             if not defined, "real tag name"
                                             is used as ordinal.
  mailnews.tags.tag-01.tag = "ppp-tag-01" <- display name(label of tag).
                  A                          Can be changed by "Rename".
                  |
                  +-- real tag name, special character is removed from
                      "display name" upon first tag definition.
                      local folder : written in X-Moziila-Keys: header of mail
                      IMAP : stored as flag of IMAP(user defined keyword)
                      due to this, real tag name is not changeable.

UI for above "ordinal" is already available by SeaMonkey.
  by "Raise Importance(move up)/"Lower Importance"(move down) interface at tag
  list panel. if tag pqr-01 is moved to before a tag, ordinal=pqr-01a, pqr-01b,
  ... is set for subsequent tags.
You can see it by download of SM2 win32.zip build and UNZIP only.
However, such UI is not implemented for Tb yet. 
If you want to order by "display name", add mailnews.tags.xxx.ordinal in prefs.js for each tag, please.

As number key of 1-9 is assigen to top most 9 tags, please don't change ordinal of $label1 to $label5(for Important, Work, Later etc.), if you want to use predeined $labelN.

As "real tag name" can not be changed once defined, I recommend you not to change tag name after tag definition frequently once "real tag name" was written in X-Mozilla-Keys: header, in order to avoid your confusion when you see mail source.
Currently, I think next is best practice for avoiding confusion by rename, if change of a few profiles only is sufficient.
  1. tag-01 is defined. xxx-tag-01 is required new name.
  2. define xxx-tag-01 newly in all Tb profiles of all Tb instances.
  3. create a virtual folder(saved search folder) for all folder of all
     accounts.
  4. filter by tag-01 at the virtual folder
  5. add xxx-tag-01 to all the filtered mails,
  6. remove tag-01 from all the filtered mails.
  7. delete tag-01 in all Tb profiles of all Tb instances

Comment 8

[Daniel Kabs, reporting bugs since 2002]

@X.em: User Interface for moving tags in TB is provided by add-on "mailtweak", see bug 528034, comment #4.

Comment 9

[Wayne Mery (:wsmwk)]

(tags have never been sortable, nor alphabetical => ENH)

Comment 10

[Thomas D. (:thomas8)]

(In reply to comment #9)
> (tags have never been sortable, nor alphabetical => ENH)

Wayne, to mark this as ENH suggests the current behaviour is somehow acceptable and consistent. Clearly, that's wishful thinking.

There might be a technical and historical explanation for the current behaviour, but that's quite irrelevant from a user's point of view. Just try to translate comment 7 for the average user and see that the legacy of the old tagging system is simply a nightmare in terms of UX-you-name it (discovery, consistency, simplicity etc.).

We really need to look at this from the user's perspective, and the actual user experience is described in my comment 2 with detailed STR:
From the user's POV, newly created tags (except the first 5) *do* get sorted in alphabetically according to the tag's display name. So the user has all reason to assume we sort by the tag's display name, and he has no way of knowing that technically, we don't, and instead sort by an invisible and not-accessible-via-UI criterion instead (the mind boggles at the thought!). Therefore, again from the user's POV, there is no reason why alphabetical order should break upon renaming - this bug. For the average user, the behaviour is simply unpredictable, thus clearly broken. Let's not pretend it isn't.

Comment 11

[Thomas D. (:thomas8)]

(In reply to comment 10)
When I say *old* tagging system, that's opposed to a modern tagging system that actually deserves the name, like the one of Firefox. That's tag-on-the fly by typing in (new) tags (with autocomplete for existing ones), have as many tags you like (and still managable), rename them as you like, and search them efficiently, again preferably by typing with partial matches:

Bug 370076 - Create/apply tags for messages by typing with the keyboard / Port Firefox tagging UI to Thunderbird (implement new way of tagging, method, applying tags too complicated)

Gmail also has some nice tagging features that would be desirable for TB:
Bug 520560 - A label system like Gmail -with new features

Comment 12

[Daniel Kabs, reporting bugs since 2002]

Great to see that you are back, Thomas D. and giving new input to TB's almost abandonded tagging system.

Comment 13

[Javi Rueda :javirid]

Attachment: 440368.patch

This patch fixes the problem by rebuilding the items on the list. While rebuilding, we sort them.

When user wanted to create a new tag, an additional step is needed in order to select the tag that has just been added.

Comment 14

[Magnus Melin [:mkmelin]]

Comment on attachment 8504336 [details] [diff] [review]
440368.patch

Review of attachment 8504336 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/components/preferences/display.js
@@ +212,5 @@
>    },
>  
> +  rebuildTagList: function() {
> +    for (let i = this.mTagListBox.getRowCount(); i >= 0; i--) {
> +     this.mTagListBox.removeItemAt(0);

does this really work? usually you'd remove item "i"

@@ +270,5 @@
> +  var item = tagListBox.getItemAtIndex(0);
> +  var i = 0;
> +
> +  while (item.label !== aName && i <= tagListBox.getRowCount() - 1
> +         && item.style.color !== aColor) {

this would be much more readable as a for-loop

Comment 15

[Javi Rueda :javirid]

Mmmm... I think I have being using for-loops and while-loops wrongly on each place.

Comment 16

[Javi Rueda :javirid]

Oops!

Comment 17

[Javi Rueda :javirid]

Attachment: Tags not alphabetically sorted in if I rename the tag (Message>Tag-Menu/Tag-Button, Tools>Options>Display>Tags)

Comment 18

[Javi Rueda :javirid]

Attachment: patch 1.1

Previous patch walked all tags at the listbox. Now it will exit the for loop the moment it finds the tag it was looking for

Comment 19

[Magnus Melin [:mkmelin]]

Comment on attachment 8514543 [details] [diff] [review]
patch 1.1

Review of attachment 8514543 [details] [diff] [review]:
-----------------------------------------------------------------

Ah, I'd actually reviewed this earlier, but forgot to push submit :(

::: mail/components/preferences/display.js
@@ +214,5 @@
> +  rebuildTagList: function()
> +  {
> +    // Remove all items at the tag box.
> +    while (this.mTagListBox.getRowCount() > 0) {
> +     this.mTagListBox.removeItemAt(0);

I would not recommend .removeItemAt(0). There's not any other hit of that on mxr.
(I guess it works, but that's error prone as you have to remember what kind of list it is. For normal lists it obviously wouldn't work.)

Maybe

while (this.mTagListBox.firstChild) {
  this.mTagListBox.removeChild(this.mTagListBox.firstChild);
}

... or remove in a for loop, removing item i, starting from last

@@ +271,4 @@
>    var tagListBox = document.getElementById("tagList");
> +  var item;
> +
> +  for (var i = 0; i <= tagListBox.getRowCount() - 1; i++) {

for (var i = 0; i < tagListBox.getRowCount(); i++)

 is shorter, and more common

@@ +273,5 @@
> +
> +  for (var i = 0; i <= tagListBox.getRowCount() - 1; i++) {
> +    item = tagListBox.getItemAtIndex(i);
> +    if (item.style.color == aColor && item.label == aName)
> +      break; // Stop the iteration as we have found the item we were looking for

I'd prefer just moving the 3 rows that do stuff with item in here. Then you don't have to declare it outside the loop either.

Comment 20

[Javi Rueda :javirid]

Attachment: Tags not alphabetically sorted in if I rename the tag (Message>Tag-Menu/Tag-Button, Tools>Options>Display>Tags)

Comment 21

[Javi Rueda :javirid]

Attachment: Patch version 1.2.1

Previous patch wasn't tested before send, sorry. This one just uses code from previous comment by :mkmelin (thank you for the advise)

Comment 22

[Magnus Melin [:mkmelin]]

Comment on attachment 8520858 [details] [diff] [review]
Patch version 1.2.1

Review of attachment 8520858 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good, thx!
However, the tag order under "Message | Tag" is still wrong.
It would also be nice if after rename, the renamed tag would scroll into view and maybe be selected.

::: mail/components/preferences/display.js
@@ +275,5 @@
> +    if (item.style.color == aColor && item.label == aName) {
> +      tagListBox.ensureElementIsVisible(item);
> +      tagListBox.selectItem(item);
> +      tagListBox.focus();
> +      break;                        // Stop the iteration as we are done

no need for the multi space. 
// we're done

is enough. (or if you want the full sentence, add a period too.)

Comment 23

[Javi Rueda :javirid]

(In reply to Magnus Melin from comment #22)
> However, the tag order under "Message | Tag" is still wrong.
> It would also be nice if after rename, the renamed tag would scroll into
> view and maybe be selected.

Menu items are ordered based on their priority assigned. I think that it would look weird if there were ordered alphabetically but access keys would be un-ordered (they are numbers).

Next patch is going to include your latest comments and the selection of the list-box item after renaming.

Comment 24

[Magnus Melin [:mkmelin]]

Ah yes... So the premises for this bug are a a bit different than I realized :( 
We should probably not sort alphabetically at all then.

Comment 25

[Javi Rueda :javirid]

Fine. I am not allowed to mark it as WONTFIX. (I am, but must not do it)

Comment 26

[Magnus Melin [:mkmelin]]

Sorry for not noticing it earlier!

If we sorted alphabetically we'd have a mess w/ priority. And adding UI for priority would also be messy for technical reasons (comment 7).