524662 - Customize Headers window is prompted twice (Creating/adding customized filter header or removing it requires to click 'OK' twice)

Status: RESOLVED FIXED

(MailNews Core :: Filters, defect)

Description

[[:Aureliano Buendía]]

STR:

1. open a new saved search window using context menu on specific folder;
2. in criterias area select "customize" for open "Customize Headers" window;
3. fill "new message header" with e.g X-Bugzilla-Status and then click Add button;
4. click OK button for unload window: the window is already prompted to user.

It seems to happens also the first time that this operation is raised from a specific folder.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.5pre) Gecko/20091026 Lightning/1.0pre Shredder/3.0pre ID:20091026031601

Comment 1

[:aceman]

It also happens in Filter management (adding custom header). The window reappears twice regardless of button pressed (OK or Cancel).
Still a problem on TB11, linux.

Comment 2

[Tristan Miller]

I can reproduce this consistently on Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20111105 Thunderbird/8.0:

Steps to reproduce:
1. Tools->Message Filters….  A "Message Filters" dialog appears.
2. New….  A "Filter Rules" dialog appears.
3. Subject->Customize….  A "Customize Headers" dialog appears.
4. If desired, add or remove a message header.  (This step is optional.)
5. Hit OK or Cancel.

Expected behaviour:
6. The "Customize Headers" dialog should disappear, and focus should return to the "Filter Rules" dialog.

Observed behaviour:
6. The "Customize Headers" dialog reappears.  This time, clicking OK or Cancel will close the dialog.  The bug will no longer occur if you repeat from step 3.  However, the bug will occur again if instead you hit OK or Cancel to close the "Filter Rules" dialog, and then repeat from Step 2.

Comment 3

[:aceman]

For some reason, this code is executed twice in /mail/base/content/mailWidgets.xml:
     <property name="parentValue">
        <setter>
          <![CDATA[
            if (this.searchAttribute == val && val != Components.interfaces.nsMsgSearchAttrib.OtherHeader) return val;
            this.searchAttribute = val;
            this.refreshList(true); // don't restore the selection, since searchvalue nulls it
            if (val == Components.interfaces.nsMsgSearchAttrib.OtherHeader)
            {
              window.openDialog('chrome://messenger/content/CustomHeaders.xul', "", 'modal,centerscreen,resizable,titlebar,chrome', null);
              setTimeout(UpdateAfterCustomHeaderChange, 0); // XXX bug 212625
            }
            else if (val == Components.interfaces.nsMsgSearchAttrib.AgeInDays)
            {
              // We want "Age in Days" to default to "is less than".
              this.value = Components.interfaces.nsMsgSearchOp.IsLessThan;
            }
            return val;
          ]]>
        </setter>

I could not yet find out from where this is called.

Comment 4

[Tristan Miller]

Confirming bug still exists on Mozilla/5.0 (X11; Linux x86_64; rv:9.0) Gecko/20111222 Thunderbird/9.0.1

Comment 6

[:aceman]

And after the customize dialog is closed, it should probably select one of the added headers (if it can be determined).

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: patch, v1

What's going on here:

The callstacks for both windows:
#0: function set_parentValue() in <chrome://messenger/content/mailWidgets.xml> line 1703
#1: function set_value() in <chrome://messenger/content/mailWidgets.xml> line 1426
#2: function onSelect() in <chrome://messenger/content/mailWidgets.xml> line 1534
#3: function oncommand() in <chrome://messenger/content/FilterEditor.xul> line 1

#0: function set_parentValue() in <chrome://messenger/content/mailWidgets.xml> line 1703
#1: function onSelect() in <chrome://messenger/content/mailWidgets.xml> line 1539
#2: function oncommand() in <chrome://messenger/content/FilterEditor.xul> line 1

onSelect sets this.value, calling its setter which does the same targets and optargets updating like onSelect, resulting in two calls to parentValue which opens the window.

Comment 9

[Kent James (:rkent)]

Comment on attachment 721261 [details] [diff] [review]
patch, v1

Yes that makes sense. Thanks for the patch.

Could you also submit a patch for the equivalent fix in the SeaMonkey version of this code?

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: patch for SeaMonkey, v1

(In reply to Kent James (:rkent) from comment #9)
> Could you also submit a patch for the equivalent fix in the SeaMonkey
> version of this code?

Here you go. Forward the review request if necessary.

Comment 11

[[e-mail]]

Comment on attachment 722540 [details] [diff] [review]
patch for SeaMonkey, v1

[What's left of onSelect could probably be inlined into the caller...]

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: patch for Thunderbird, v2, r=rkent

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: patch for SeaMonkey, v2, r=Neil

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/9737e2f98425
https://hg.mozilla.org/comm-central/rev/c164453b2b01