140591 - Create filter from a message broken in Mozilla 1.0 branch

Status: VERIFIED FIXED

(MailNews Core :: Filters, defect)

Description

[Mark Bitterling]

Downloaded Mozilla 1.0 build 2002042605 for Mac OS X onto a brand new just out
of the box iMac running OS X 10.1.4 and created a new profile.  After retrieving
some email that had been waiting on the server, I did a right click on the
sender's email address in the header and selected "Create Filter from
Message..." from the context menu.

Expected result : Mail Filter sheet should drop down with filter set to trigger
on the sender's email address.

(Note: I actually would expect that a Mail Filter dialog would pop up rather
than a sheet since I am not really convinced that Message Filters should be
using sheets but that is a different bug and right now either would be
preferrable to current nonfunctional behavior).

Actual Result : Nothing appears.  However, two new ghost entries now exist in
the Window Menu : one is blank entry and the other is labeled "Filter Rules" --
neither of these can be brought to focus.  AFAIK, the only way to close these
ghost windows is to exit Mozilla; Mozilla does not exit cleanly though because I
get a "Mozilla has unexpectantly quit" error message.  

After restarting Mozilla, I tried creating the filter using  "Message->Create
Filter from Message..." from the menu instead of the context menu with the same
results.

So then I tried to create the filter via "Tools->Message Filters" and clicking
on the "New" button, but as long as these ghost entries remain this does not
work either.  However, if I restart Mozilla and do not try to create filter from
a message (thus no ghost entries), the "Tools->Message Filters->New" works just
fine.

I have not yet had a chance to test this on any other platforms or other
installations, but I will try to do so later today.

Comment 1

[Mark Bitterling]

Just now checked on my regular G3 workstation using Mac OS X build 2002042705
from the Mozilla 1.0 branch and encountered the same problem.

Comment 2

[Mark Bitterling]

On the same G3 using OS X build 2002041712 (the only other build I currently
have on this machine), the context menu brings up something that seems to be a
cross between a dialog and a sheet for the configuration of the filter.  This is
better than the behavior of the newer builds because it at least is functional,
but it still seems to be wrong.  

With this build, I am still seeing the blank entry and the "Filter Rules" entry
in the Windows Menu. If these are sheets then they probably should not show up
in the Windows Menu should they?  I am guessing that the blank entry is actually
for the hidden "Message Filters" sheet that drops down when the "Filter Rules"
sheet/dialog is dismissed.

Comment 3

[Mark Bitterling]

Ok, I did some more checking.  

Something changed between branch build 2002041805 and branch build 2002041905
that caused the Filter Rules to not get focus.

Looking through Bonsai, I noticed a checkin for the bug 57841 which deals with
window.focus() and window.blur() but my knowledge in this area is almost nil so
that may be totally irrelevant.  I'll leave it up to someone else to determine
what happenned.

 

Comment 4

[Greg K.]

I can sort of confirm this problem using FizzillaCFM/2002041712 (RC1).

1. Select a message header
2. Invoke the contextual menu on the "From" address
3. Select "Create Filter from Message..."

I saw the Filter Edit sheet appear disconnected from any window, which was
bizarre. Dismissing the sheet, the Filters sheet then drew attached to the
Mail/News window. It was only possible to then dismiss that sheet by clicking,
holidng, then dragging slightly on its' "OK" button.

Comment 5

[laurel]

Mac OS X only, not broken in mac OS 9.2 or win32,linux.

Comment 6

[laurel]

This works in the apr17 branch, broke sometime after that.

Comment 7

[scottputterman]

Let's get this fixed for rtm.

Comment 8

[Navin Gupta]

Attachment: proposed fix to try out on mac os X

The fix is to not use the filterListDialog to launch filterEditor for prefill
filter case but launch it directly from mail3pane. When the user hits ok, it
will
launch filterListDialog otherwise it will do nothing (as expected). This should

also fix bug 112714. 

This doesn't fix that crasher on quit, changing filterEditor to open as
non-modal
fixes that but we may have some usability issues so I'll fix that separately.

Comment 9

[Navin Gupta]

thanks to jf, I just tried this patch on his mac os x debug build and it works
fine. 

Comment 10

[(not reading, please use [e-mail] instead)]

I'd like varada to review this code.  I'd recommend looking over laurel's test 
plan for this feature, make sure you haven't forgotten about any edge cases.
(there are several ways to create a message from a filter, and scenarios for if 
the filter dialog is already open, etc.)

One question I had:

in MsgFilters(), there's a new code path where we'll call window.openDialog()
twice in a row:

+      window.openDialog("chrome://messenger/content/FilterEditor.xul", "", 
+                        "chrome,modal,resizable,centerscreen,dialog=yes", 
args);
+
+      if ("refresh" in args && args.refresh)
+      {
+         args = { folder: preselectedFolder };
+         window.openDialog
("chrome://messenger/content/FilterListDialog.xul", "", 
+                       "chrome,modal,resizable,centerscreen,dialog=yes", args);

who sets refresh on args?  (I'm guessing something in Filter*.js does it.)
is it only set when OK is hit in the filter edit dialog?

what does your code do?  Does it first open the filter edit dialog, and then 
when you dismiss it, if refresh is set, we open the filter list dialog, but if
not (on cancel) we don't do anything.

can you add some comments?  The code for creating a filter from a message could 
use more comments.

Comment 11

[varada]

the patch seems fine  add those comments that we discussed over aim also make
sure that it is ok with all cases on win and linux too with that - r= varada

Comment 12

[Navin Gupta]

Attachment: patch w/ comments

I have tested on all three platforms. I have added comments to explain what is
going on in MsgFilter(). The boundary cases have reduced significantly since
we made everything modal. 

>who sets refresh on args?  (I'm guessing something in Filter*.js does it.)
>is it only set when OK is hit in the filter edit dialog?

yes, it is only set when we hit ok in filterEditor dialog.

>what does your code do?  Does it first open the filter edit dialog, and then 
>when you dismiss it, if refresh is set, we open the filter list dialog, but if

>not (on cancel) we don't do anything.

yes, you got it right.

Comment 13

[Navin Gupta]

Comment on attachment 82981 [details] [diff] [review]
patch w/ comments

carrying fwd r=varada

Comment 14

[(not reading, please use [e-mail] instead)]

Comment on attachment 82981 [details] [diff] [review]
patch w/ comments

thanks for adding the comments and checking with varada.

sr=sspitzer

Comment 15

[Navin Gupta]

fixed on trunk

Comment 16

[Mark Bitterling]

Is there any particular reason why this has not been checked into the 1.0
branch?  Besides the fact that there is no a= ?

Also, has there been another bug openned regarding the modal issue?

Comment 17

[scottputterman]

Laurel, could you verify this on the trunk?

Comment 18

[laurel]

Verified on trunk: Mac OX 10.1 with may20 commercial trunk build.

Comment 19

[scottputterman]

adding adt1.0.0+.  Please get drivers approval and then check into the 1.0 branch.

Comment 20

[scottputterman]

changing to adt1.0.1+ for checkin to the 1.0 branch for the Mozilla1.0.1
milestone.  Please get drivers approval before checking in.

Comment 21

[laurel]

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

Comment 22

[laurel]

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

Comment 23

[scottputterman]

adding mozilla1.0.1 for asking for drivers approval.

Comment 24

[Navin Gupta]

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

Comment 25

[Judson Valeski]

please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.

Comment 26

[Navin Gupta]

fixed on branch

Comment 27

[laurel]

OK using june13 commercial branch build on Mac OS 10.1

Comment 28

[laurel]

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

Comment 29

[laurel]

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