257885 - Setting a Flag/Star with the context menu causes the Mark|Flagged checkmark to persist incorrectly

Status: VERIFIED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Christoffer Enedahl]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040803 Firefox/0.9.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040803 Firefox/0.9.3

In news using threadview, flag a responsemessage, now if you want to flag the
above message the rightclick menu indicates that is't already flagged but if you
click mark>flag it works as if it was not flagged (as it should).

Reproducible: Always
Steps to Reproduce:
1.In news using threadview, flag a responsemessage
2.Rightclick the above message
3.Walk in menu mark>flag

Actual Results:  
The menu showed that it was flagged

Expected Results:  
The menu should not be flagged.

Comment 1

[Mike Cowperthwaite]

This is not specific to news, and not specific to threaded mode, and not limited 
to the parent, to messages in the thread, or even to messages in the same group 
or folder.

If you use the message context menu to set the flag, that checkmark will 
continue to be set until you use the context menu to *clear* a flag (not 
necessarily the same flag you set).  Once a flag has been cleared, the context 
menu's checkmark will show the status appropriate to the message.

I see this in TB 0.7-Win2K.  Leaving unconfirmed until I can check against a 
newer version.  (Christoffer Enedahl, please remember to include your TB build 
ID when you open a bug.)

Comment 2

[Mike Cowperthwaite]

Problem persists in TB 0.8.

Comment 3

[Mike Cowperthwaite]

*** Bug 274014 has been marked as a duplicate of this bug. ***

Comment 4

[zug_treno]

I can also reproduce this bug in Mozilla 1.8b1.

Comment 5

[anonymous]

Still persists in Thunderbird 1.0.2 (20050317)

Comment 6

[Yann Coupin]

I noticed this when doing the Q&A for thunderbird 1.5 RC2

Comment 7

[Yann Coupin]

(In reply to comment #6)
> I noticed this when doing the Q&A for thunderbird 1.5 RC2
> 

Forgot to mention it was on win2k

Comment 8

[[:Aleksej]]

Still (and also) happens with
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a2pre) Gecko/20070121 Thunderbird/3.0a1 ID:2007012104
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.2pre) Gecko/20070116 Thunderbird/2.0b2 ID:2007011615

Comment 9

[Carsten Book [:Tomcat]]

changing to OS= all because of comment #8

Comment 10

[LT]

Nobody's confirmed this for a release build of 1.5, so I'm verifying this on 1.5.0.9 (20061207) as well.

It's really hard to flag messages when they show up as already flagged and you have to click repeatedly until you're sure the message is actually flagged (or not). :(

If you click 'Flag' on the right-click menu of an individual message, it updates the menu correctly for the next time you right-click, so it has to be something simple like moving to a different message not updating that field/variable/however it's stored. Clearing that flag and resetting it when moving between messages should take care of it- something must have just been missed there.

Comment 11

[LT]

Also, I'm guessing this is basically the same issue as the following bugs (Mark as Read doesn't always show correctly)?

Bug 64326
Bug 223073
Bug 321651
Bug 347150


The multiple-selection thing doesn't apply to this particular bug, although the same problem with selecting multiple messages occurs when using the Mark as Flagged menu item.

Comment 12

[Scott MacGregor]

This is a very interesting bug and I suspect it effects seamonkey too. The various menuitems in the thread pane context menu, message pane context menu, mark toolbar button menu popup and the mark menu item that let you flag a message all observe the command: cmd_markAsFlagged. 

When these menus are opened, we go through InitMessageMark():

http://lxr.mozilla.org/mozilla/source/mailnews/base/resources/content/mailWindowOverlay.js#639

which sets the checked attribute on the command element, with the intent that the menu items observing this command pick up this attribute. If you go to another message that isn't flagged, all the menu items reflect the correct state except for the menu item used to flag the previous message. In DOM inspector I see that the command element has the checked attribute set to false, but the menu item now has its own checked property which is true. I think the menu item is setting its own checked attribute in addition to the value set on the command since it is a menu item of type checkbox.

This same situation applies to the mark as read command. 

One possible solution is to pass in the actual dom elements for the flag and unread menu items into InitMessageMark. Or maybe there's a way to prevent the menuitem from setting the check state. Neil, do you have another suggestion on how we could fix this?

Comment 13

[[e-mail]]

If a menuitem has a command attribute and the corresponding command element has a checked attribute then that attribute should be copied to the menuitem when the popup opens (see nsMenuFrame::OnCreate). In particular it should definitely be copying the checked="false" attribute in the case you describe.

Comment 15

[Scott MacGregor]

(In reply to comment #13)
> If a menuitem has a command attribute and the corresponding command element has
> a checked attribute then that attribute should be copied to the menuitem when
> the popup opens (see nsMenuFrame::OnCreate). In particular it should definitely
> be copying the checked="false" attribute in the case you describe.
> 

I am seeing in dom inspector that the command has value of false for the checked attribute while the menu item that is observing that command has its own checked attribute with a value of true.

Comment 16

[Scott MacGregor]

This actually works in seamonkey, so I think Thunderbird is probably doing something wrong rather than a toolkit level problem.

And sure enough there is a difference in the menu items.

Thunderbird:

<menuitem id="threadPaneContext-markFlagged"
          type="checkbox" label="&markStarredCmd.label;" accesskey="&markStarredCmd.accesskey;"
          observes="cmd_markAsFlagged"/>

Seamonkey:

<menuitem id="threadPaneContext-markFlagged"
          type="checkbox" label="&markStarredCmd.label;" accesskey="&markStarredCmd.accesskey;"
          command="cmd_markAsFlagged"/>

Thunderbird is using observes where seamonkey converted all of the observes attributes over to command over in: https://bugzilla.mozilla.org/show_bug.cgi?id=288366#c6

And sure enough making this change fixes the problem for me.

Comment 17

[Scott MacGregor]

Attachment: use command instead of observes

This ports the seamonkey change in Bug 288366 to use command instead of observes.

This ends up fixing the problem for Star and Mark As Read.

Comment 18

[[e-mail]]

Strange, observes should take effect immediately (command only takes effect because the menu is being shown).

Comment 19

[Mike Cowperthwaite]

(In reply to comment #18)
> Strange, observes should take effect immediately (command only takes effect
> because the menu is being shown).
 
"Immediately" meaning when the selection is changed?  When the message state changes?

Comment 20

[[e-mail]]

"Immediately" meaning when the code sets the attribute on the element being observed. Most of the time command= is equivalent to observes=. However there are a few exceptions:
1. When using command= any command event is fired on the command element.
2. When using <key command> the key never observes any attributes.
3. When using <menuitem command> the menuitem only observes certain attributes and then only between its popupshowing and popupshown events.

Comment 21

[Mike Cowperthwaite]

Scott, this is checked in trunk & branch, isn't it?

(xref regression at bug 370183.)

Comment 22

[Scott MacGregor]

I must have forgotten to mark this fixed. thanks mcow.

Comment 23

[Mike Cowperthwaite]

V, for Star and Read, with TB 2b2-0219.