40356 - Filter/Search UI: remove space between criteria columns

Status: RESOLVED WORKSFORME

(MailNews Core :: Filters, defect, P3)

Description

[laurel]

Using may23 m16 commercial build

Within the Filter Rules dialog the leftmost column is a notation of the AND or
OR selection they've made in conjunction with the criteria line(s) they've
added.  This display is not working quite right.

Scenario #1 (column doesn't display when user sets AND/OR radio button):
1.  From mail window, Edit|Message Filters and click New button.
2.  Within the filter rules dialog, type a name, click either the AND or OR
radio button ("Match ALL..." or "Match at least ONE..."). Click the More button
to display first (default) criteria line.
3.  Do not change selection in the criteria widgets, leave them at Subject
Contains and type some text in the text field for that line.
4.  Click the More button. Change Subject to Sender, leave Contains as is, and
type text in that line's text field.
5.  Click the More button again.

Result:  When you preset the radio button before adding criteria lines, nothing
appears in the left column of each criteria line.  It should appear as follows:
    First criteria line:  "the" 
    All other criteria lines:  "or the" if OR radio button selected  "and the"
if AND radio button selected.

Scenario #2 (column for first criteria line display's incorrectly):
1. From mail window, Edit|Message Filters and click New button.
2. Within the filter rules dialog type a filter name, but DON'T click either of
the AND/OR radio buttons.
3. Click the More button to expose criteria line and see the left column of the
first criteria line says "or the" when it should just say "the". 
4. Add more criteria lines. In some cases it will display correctly "or the" in
that column, sometimes it will be blank.  
5. Click the radio button to AND.  Text changes to and... when it should be
"the" for the first line, but will most often display right on subsequent
criteria lines.

Result:  The first line AND/OR column is always incorrect.  Behavior is
inconsistent for the subsequent lines if you didn't preset the radio button.

QA note:  when fixed, there are several variations to test to make sure this
AND/OR notation initially displays correctly, changes according to change in
radio button setting, displays correctly when adding and decreasing criteria
lines.

Comment 1

[laurel]

Additional case to try:  saw a case where And mixes with Or in one filter rules
dialog, And on one line, or on another.  Situation was with migrated AND filters
where I opened a filter to edit in seamonkey, clicked More to add a line and it
came up OR.

Comment 2

[Alec Flett]

I don't expect to fix this for beta2, but it's a FCS must-fix.

Comment 3

[scottputterman]

adding keyword b3mail

Comment 4

[selmer (gone)]

Adding nsbeta3 to b3mail bugs.

Comment 5

[selmer (gone)]

Removing b3mail keyword (these bugs have been promoted now.)

Comment 6

[laurel]

QA note: Also applies for search messages dialog. Verify both when fixed.

Comment 7

[Alec Flett]

reassigning my filter bugs to gayatrib..

Comment 8

[laurel]

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

Comment 9

[scottputterman]

reassigning to naving

Comment 10

[Håkan Waara]

This bug is not valid any more, because now the label before the tree says
"Match all of the following" and "Match any of the following", and thus we don't
need the and/or column imho.

So, to morph this bug, I'd like to remove all the additional space in this
search/filter tree, and make the widgets within it use the as much of the space
as possible.  It looks quite nice, see the screenshot.  I would also like to
know if jglick is OK with removing and/or from the spec, in favor of the
upcoming screenshot (I have a patch).

Comment 11

[Håkan Waara]

Attachment: Screenshot

Note the enhanced criteria tree.

Comment 12

[jglick]

Since there doesn't seem to be any chance of the "the/or the * the/and the" text 
getting fixed anytime soon, I agree its better to clean up the dialog as Håkan 
suggests. Bug 82015 is similar to this bug and needs to be fixed as well. 

Comment 13

[Håkan Waara]

Attachment: Patch

I have tested the UI, everything looks fine.  It's looks nice without the ugly
space in the tree.

Comment 14

[Christopher Aillon (sabbatical, not receiving bugmail)]

Since you're removing its users, you can also remove these two lines:

         var stringBundle = this.stringBundle;
         var andString = val ? "And" : "Or";

Do that and r=caillon.

Comment 15

[Håkan Waara]

Thanks for the review.  Lines removed in my tree.  CC Seth for sr=...

Comment 16

[Håkan Waara]

Still need sr=...

Comment 17

[Håkan Waara]

Attachment: Patch v2

Ok, so basically the fix is to:

* Make us not create nulls in our searchTerm array, so we don't create the
columns between the search terms like we had before.

* Remove handling for and/or strings in the UI, as discussed.

Comment 18

[Håkan Waara]

Attachment: Patch v2

Attaching the right patch this time too.  :-)

Comment 19

[Håkan Waara]

bienvenu/sspitzer: can you sr= my patch? it already has r=caillon.

Comment 20

[David :Bienvenu]

Navin owns the filters code, so he should be given an opportunity to review
this. He's on vacation at the moment.

Comment 21

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

a couple comments:

1)  if they are only hit in error, or unexpected exception, the dump()
statements are OK to leave in.

2)  I think these extra spaces are there for a reason.  I think in 4.x the
reason was for non english languages.  Specifically, Japanese.  I think the
problem was that while it read correctly to have:

[Sender] [Contains] [hwaara]

That didn't work for Japanese.

For Japanese it was something like:

<w> [Sender] <x> [Contains] <y> [hwaara] <z>

momoi, alecf, nhotta, do you remember that 4.x bug?

From search.properties:

# these are the fields that get inserted in the search line
# for "and" searches, this looks like:
#
# searchAnd0 <attribute> searchAnd1 <operator> searchAnd2 <value> searchAnd4
#
# for example, in english this looks like:
# and the [Sender ] [doesn't contain] [John]
#
# TODO: need to special-case the first line (filterindex==0)
#

we can also look at the localized versions of mozilla / netscape to see how / if
they are being used.

I think this might turn into an invalid bug.

Comment 22

[Katsuhiko Momoi]

Seth, Netscape 6/7 Japanese builds don't seem to use spaces to 
insert grammatical particles as we used to do for Comm 4.x.

Currently the filter dialog is a mess in Netscape JA builds.
They should learn from Mozilla-Japan builds and cut down on word length
so that the entire words may be visible. But it seems all the text are
now contained within the pull down widgets. 

But I would like some input from Ying-Lin who does L10n for Japanese
for all 3 platforms for Netscape. Ying-lin, you don't have any
plans to use the spaces inbetween the widgets, do you? (We certainly
need to improve on the dialog length in the filter dialog, however.)

Comment 23

[Håkan Waara]

I still thinks the UI looks bad in English with the spaces. Can't we
special-case the japanese builds and on runtime decide if we need the spaces?

Comment 24

[Alec Flett]

yes, that's exactly what those extra columns are for! do not remove that code -
the patches here are totally bogus and wrong. The intent is to make the dialog
more localized. I guess I should have documented more.

Comment 25

[Alec Flett]

also, the real bug here is that blank labels in the UI are getting width. they
should have a zero-width if there is no text in the label.

Comment 26

[Håkan Waara]

Good point. If we could hack the columns to have zero width when the labels
inside them are empty, we could make all parties happy, right? :-)

Comment 27

[laurel]

Nominating for cleanup in next release -- This is useless space at this point,
we should either make the AND/OR work again or remove the excess space to make
the dialog a little more polished.  Also see bug 82015.

Comment 28

[scottputterman]

+'ing to remove and/or

Comment 29

[laurel]

This really isn't an issue anymore, the space is now used with the fix in bug
171236.  Unless we're going to take out the and/or text again, the empty space
issue is moot.

Comment 30

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

shuehan, this sounds related to that bug you were working on.

Comment 31

[jglick]

and/or removed with fix in bug 183994.