870313 - Save drop-down on toolbar button of Composition window loses tick after first save

Status: RESOLVED WORKSFORME

(Thunderbird :: Message Compose Window, defect)

Description

[Mark Banner (:standard8)]

Bug 507103 added a tick to the menu drop-down on the save button in the composition window.

When I first open the window the tick is on draft. As soon as I've saved something, the tick is lost.

If I look at the option in the File -> Save As menu, the tick is retained.

Comment 1

[Thomas D. (:thomas8)]

Mark, wfm on WinXP / Daily 23.0a1 (2013-05-04), radio checkmark always in the right place on both popup menus.

Can you add precise STR?
Do you see this in Safe-Mode?

Comment 2

[WADA:World Anti-bad-Duping Agency]

(In reply to Mark Banner (:standard8) from comment #0)
> When I first open the window the tick is on draft. As soon as I've saved something, the tick is lost.
> If I look at the option in the File -> Save As menu, the tick is retained.

Checked with following trunk nightly on Win-XP, with no add-on relevant to mail composition.
> Mozilla/5.0 (Windows NT 5.1; rv:23.0) Gecko/20100101 Thunderbird/23.0a1
> Application Build ID : 20130509030652

(A) Within an compose window
    (A-1) Open a compose window, call WIN-1
    (A-2) Do Save operation, changing File/Draft/Template of Drop Down.
          => selected action is executed as expected
          Call selected one "XXX". (tested with "Template")
    (A-3) Do Save operation, without showing Drop Down menu
          => Action of XXX is executed as expected
    (A-4) Show Drop Down menu of Save
          => Action of XXX is still shown as "Selected"
    (A-5) Hide and Show again Composition ToolBar, Menue Bar etc.,
          Show Drop Down menu of Save of Menu Bar/Composition ToolBar
          => Action of XXX is still shown as "Selected"
    (A-6) Do Save operation, without showing Drop Down menu
          => Action of XXX is executed as expected
    (A-7) Repeat A-2 to A-5
          => selected action of XXX is retained in WIN-1
          => selected action of XXX is executed as expected at WIN-1
    => Change by Bug 507103 works pretty well in my test.  

(B) At second compose window (new window, not recycled window)
    (B-1) Open second compose window, call WIN-2
          => "Draft" is set as pre-selected one in Drop Down menu
    (B-2) Repeat same operation as A-2 to A-7 at WIN-2
          => selected action of XXX is retained in WIN-2
          => selected action of XXX is executed as expected at WIN-2
    Because each composition window is independent,
    and status of "selected Save As option" is saved in XUL element
    in a compose window, this is pretty normal.

(C) By composition window recycle.
    (C-1) With mail.compose.max_recycled_windows > 0,
          and with build after fix of Bug 866223(FIXED on 2013-05-01),
          Open a new compose window, call WIN-X,
          change "Save As" selection to oter than Draft,
          close WIN-X => held as cached composition window
    (C-2) Open a compose window again. WIN-X is re-used.
          Initial selection at "Save As" == Draft
     => Perhaps one of next;
        (i)  Selection status of "Save As" is not retained
             when closed compoition window is cached.
        (ii) Selection status of "Save As" is initialize to
             internal default of "Draft" upon composition window open,
             regrdless of last selection before recycle.

If more than two compose windows are opened at same time, we can see any of "File ticked", "Draft ticked", "Template ticked" at a compose window.
Mark Banner, do you see (B-1) or (C-2)?
What phenomen do you call "the tick is lost"?
- "Diferent one from previous selection" was ticked
- Nothing is ticked
How was composition opened?
- Compose/Reply/Forawar menu/button etc. of Tb
- Sendto/Mail Recipient of Windows Explorer
- Send file as mail by app lik Word, Excel
- Other
If other than "open is initiated by Tb's feature", was Tb already running? Or Tb is not running and composition window only was opened?

Comment 3

[WADA:World Anti-bad-Duping Agency]

(Correction of my comment #2)
>     and status of "selected Save As option" is saved in XUL element

It was wrong.
"defaultSaveOperation" was JavaScript global variable of DOM Window for a mail composition window(window.defaultSaveOperation).
When cached window is re-used, "Onreopen" is notified to event listener.
> http://mxr.mozilla.org/comm-central/source/mailnews/compose/src/nsMsgComposeService.cpp#263
> 258           nsCOMPtr<nsIDOMWindow> domWindow(mCachedWindows[i].window);
> 259           nsCOMPtr<nsIXULWindow> xulWindow(mCachedWindows[i].xulWindow);
> 260           rv = ShowCachedComposeWindow(domWindow, xulWindow, true);
> 261           if (NS_SUCCEEDED(rv))
> 262           {
> 263             mCachedWindows[i].listener->OnReopen(params);
> 264             return NS_OK;
And at re-opened DOM window, "defaultSaveOperation" is re-initialized to "draft" by following routine when cached composed window is re-used.
> http://mxr.mozilla.org/comm-central/source/mail/components/compose/content/MsgComposeCommands.js#252
> 252   onReopen: function(params) {
> 253     InitializeGlobalVariables();
http://mxr.mozilla.org/comm-central/source/mail/components/compose/content/MsgComposeCommands.js#111
> 111 function InitializeGlobalVariables()
> 120   defaultSaveOperation = "draft";

Value of "defaultSaveOperation" is changed by each oncommand script for Save File/Save as draft/Save as Template. 
However, "Change of checked status of radio buttons for File, Draft, Template in Drop Down menu of XUL element" is done by onpopupshowing script for the Drop Down menu.
> onpopup"defaultSaveOperation"
> +         <menupopup id="button-savePopup" onpopupshowing="InitFileSaveAsMenu();">
So, if someone checks "checked" status of <menuitem> for File/Draft/Template without expanding the Drop Down menu, or if someone tries to change image for "Save" button by CSS based on attribute value/status without using window.defaultSaveOperation value before expanding the Drop Down menu, it looks that the "latest selection of File/Draft/Template" is lost.

Are you talking about this phenomenon?

If so, function InitFileSaveAsMenu(), which is called by onpopupshowing, should be called also in each command="cmd_saveAsFile", command="cmd_saveAsDraft", command="cmd_saveAsTemplate".

Comment 4

[WADA:World Anti-bad-Duping Agency]

Although window.defaultSaveOperation is shared by submenu of File/Save As menu and Drop Down menu of Save button, "submenu for File/Draft/Template" is defined independently under File/Save As menu and Drop Save Botton menu. So, if "change at File Save As menu" should be immediately reflected to "Save Button menu" or vice versa, each command="xxx" should update "Checked" status of drop down menus under other parent's menu.

Another way to resolve this kind of problem.
- Define menu under <popupmenu> as common menu and share it among
  multiple paret menus.
- In popupshowing script, show the common drop down at side of each
  parent menu.

Comment 5

[Thomas D. (:thomas8)]

(In reply to WADA from comment #3)

> So, if someone checks "checked" status of <menuitem> for File/Draft/Template
> without expanding the Drop Down menu, or if someone tries to change image
> for "Save" button by CSS based on attribute value/status without using
> window.defaultSaveOperation value before expanding the Drop Down menu, it
> looks that the "latest selection of File/Draft/Template" is lost.
> 
> Are you talking about this phenomenon?

Independent of what Mark is seeing, I agree with WADA that it's programmatically better/cleaner to synchronize the menus immediately when cmd_saveas_flavor1/2/3 is called, rather than later with onpopupshowing.

> If so, function InitFileSaveAsMenu(), which is called by onpopupshowing,
> should be called also in each command="cmd_saveAsFile",
> command="cmd_saveAsDraft", command="cmd_saveAsTemplate".

That sounds like a good direction to me, might solve Mark's problem, too.

Comment 6

[:aceman]

So what about asking the author of patch in bug 507103 ? :)

Comment 7

[Thomas D. (:thomas8)]

(In reply to :aceman from comment #6)
> So what about asking the author of patch in bug 507103 ? :)

Sakshi, aceman is suggesting that you could tweak your existing patch of bug 507103 such that we update the menus earlier when one of the three save commands are triggered instead of only before the submenu popup is shown by user, see comment 5.

Comment 8

[:aceman]

Patch for bug 507103 is checked in, so it can't be tweaked. I just thought that its author can fix this bug too (with a new patch) as he already knows the code.

Comment 9

[Thomas D. (:thomas8)]

(In reply to :aceman from comment #8)
> Patch for bug 507103 is checked in, so it can't be tweaked. I just thought
> that its author can fix this bug too (with a new patch) as he already knows
> the code.

Yep; I realized my unfortunate wording just after commenting, was meant to be "build on", "continue from" or something like that as we are not starting from scratch. As for Sakshi, let's say "she" when we refer to her ;)

Comment 10

[Mark Banner (:standard8)]

The only thing I can reproduce currently is:

1) Start up Thunderbird
2) Select Write
3) On the toolbar, select the drop-down menu next to save

=> Draft option is ticked (correct)

4) Select the template option
5) Show the drop-down menu again

=> No options are ticked (wrong)

6) Go to File -> Save As

=> Template option is ticked (correct)

7) Save as a draft
8) View the drop-down next to the save button on the toolbar

=> Draft option is ticked (correct)

Comment 11

[Thomas D. (:thomas8)]

(In reply to Mark Banner (:standard8) from comment #10)
> The only thing I can reproduce currently is:
> 4) Select the template option
> 5) Show the drop-down menu again
> => No options are ticked (wrong)

I can't reproduce that on WinXP/TB 24.0a1 (2013-06-21) - wfm.
Mark, on which OS/TB version are you seeing this?
Do you also see this in Safe-Mode?

Comment 12

[:aceman]

I can't see this either. Jorg, have you come across this?

Comment 13

[Jorg K (CEST = GMT+2)]

I can't see a problem here, last selected option sticks.

Comment 14

[u669947]

Hello can i work on this issue?

Comment 15

[Falguni Islam]

Hello, I am new to contributing to Bugzilla can you assigned me to this issue so that I can learn to fix these bugs and also suggest to me how can Is start working on this bug to fix it.

Comment 16

[Mark Banner (:standard8)]

Sorry, something here has fixed itself in the meantime. This seems to be working fine on the latest Thunderbird release.