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

VERIFIED FIXED

Status

Thunderbird
Mail Window Front End
--
minor
VERIFIED FIXED
14 years ago
11 years ago

People

(Reporter: Christoffer Enedahl, Assigned: Scott MacGregor)

Tracking

({fixed1.8.1.3})

x86
All
fixed1.8.1.3
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

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

14 years ago
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.)
Severity: trivial → minor
Summary: Flag a re: message in news-threadview makes parent's rightclickmenu showing faulty flag indication → Setting a Flag with the context menu causes the Mark|Flagged checkmark to persist incorrectly

Comment 2

13 years ago
Problem persists in TB 0.8.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 3

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

Comment 4

13 years ago
I can also reproduce this bug in Mozilla 1.8b1.

Comment 5

13 years ago
Still persists in Thunderbird 1.0.2 (20050317)

Comment 6

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

Comment 7

12 years ago
(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

Updated

12 years ago
Version: unspecified → 1.5

Updated

12 years ago
Summary: Setting a Flag with the context menu causes the Mark|Flagged checkmark to persist incorrectly → Setting a Flag/Star with the context menu causes the Mark|Flagged checkmark to persist incorrectly

Comment 8

11 years ago
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
changing to OS= all because of comment #8
OS: Windows XP → All

Comment 10

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

11 years ago
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.
(Assignee)

Comment 12

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

11 years ago
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.

Updated

11 years ago
Duplicate of this bug: 347150
(Assignee)

Comment 15

11 years ago
(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.
(Assignee)

Comment 16

11 years ago
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.
(Assignee)

Comment 17

11 years ago
Created attachment 254469 [details] [diff] [review]
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.
Attachment #254469 - Flags: superreview?(bienvenu)

Updated

11 years ago
Attachment #254469 - Flags: superreview?(bienvenu) → superreview+

Comment 18

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

Comment 19

11 years ago
(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

11 years ago
"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.
(Assignee)

Updated

11 years ago
Depends on: 369936

Updated

11 years ago
Depends on: 370183

Comment 21

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

(xref regression at bug 370183.)
(Assignee)

Comment 22

11 years ago
I must have forgotten to mark this fixed. thanks mcow.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Keywords: fixed1.8.1.3
Resolution: --- → FIXED

Comment 23

11 years ago
V, for Star and Read, with TB 2b2-0219.
Status: RESOLVED → VERIFIED

Updated

11 years ago
Duplicate of this bug: 223073
You need to log in before you can comment on or make changes to this bug.