Last Comment Bug 765637 - (tags-concatenation) Selecting multiple tags in quickfilter/view-button-menu should allow AND-operation
(tags-concatenation)
: Selecting multiple tags in quickfilter/view-button-menu should allow AND-oper...
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Search (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: Thunderbird 24.0
Assigned To: Magnus Melin
:
Mentors:
Depends on:
Blocks: tb-tagsmeta
  Show dependency treegraph
 
Reported: 2012-06-17 18:20 PDT by Dominik
Modified: 2013-09-11 15:32 PDT (History)
12 users (show)
mkmelin+mozilla: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
proposed fix (4.48 KB, patch)
2013-02-13 12:43 PST, Magnus Melin
bwinton: ui‑review-
Details | Diff | Review
screenshot with all/any button (35.40 KB, image/png)
2013-02-16 13:06 PST, Magnus Melin
bwinton: ui‑review+
Details
Design of tag button with inclusion (plus) and exclusion (minus) activated (11.92 KB, image/jpeg)
2013-02-17 10:07 PST, Dominik
bwinton: ui‑review-
Details
proposed fix, v2 (20.73 KB, patch)
2013-03-03 04:33 PST, Magnus Melin
bwinton: ui‑review+
Details | Diff | Review
screenshot (34.51 KB, image/png)
2013-03-03 04:35 PST, Magnus Melin
no flags Details
proposed fix, v3 (20.96 KB, patch)
2013-03-25 13:29 PDT, Magnus Melin
mkmelin+mozilla: ui‑review+
Details | Diff | Review
proposed fix, v4 (20.78 KB, patch)
2013-04-06 03:37 PDT, Magnus Melin
bugmail: review+
mkmelin+mozilla: ui‑review+
Details | Diff | Review

Description Dominik 2012-06-17 18:20:27 PDT
Concatenation of Tags:

If I click/activate two (or more) tags in the quick filter toolbar, then it should concatenate the two tags so that only mails which have both tags are displayed. This would be the (intuitive) behavior which is known from web 2.0-websites like stackoverflow.com or delicious.com . This is also expected in a UI suggestion [1].

-> Current behaviour is:

If I click/activate two tags (tag1 & tag2) in the quick filter toolbar, mails which have either tag1 or tag2 are displayed.


Exclusion of Tags:

It should also be possible to exclude tags so that only mails which don't have that specific tag are displayed. This feature is quite useful. You can see this in use for example on freecode.com with filtering free software  by excluding tags like "proprietary", "shareware" etc. instead of including all the free software licenses (like "GPL", "BSD", etc.) [2]. Another user also use this feature (with custom views) [3]

A suggestion for the button UI (see the plus and minus icons):

---------------
| +           |
|   Important |
| -           |
---------------


I know that both topics I mentioned can be done with creating custom views but clicking on tags is quite the more intuitive and normal way in web 2.0 (-;


The extension Seek [4] seems to be the ne plus ultra for filtering mails but unfortunately it doesn't work in Thunderbird 13.0.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=395985#c7
[2] http://freecode.com/tags/email?page=1&with=&without=2762%2C6430%2C4143
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=379465
[4] http://code.google.com/p/simile-seek
Comment 1 Thomas D. (currently busy elsewhere; needinfo?me) 2012-07-17 02:10:02 PDT
I'm quite sure I've seen this useful RFE before, but can't find the dupe.
Closest I find is Bug 395985 which is not explicitly about quickfilter.
Comment 2 Dominik 2013-02-07 14:34:49 PST
Exclusion of Tags: 

Could be done by right-clicking on the button (if it is not currently selected). Thanks to "alta88" who mentioned it in the description of the addon "QuickFilterToolbar" [1].
However, it's more than half a year ago as I filed this bug and until now, nobody gave me a hint to this. So this is really not intuitive at all, i.e. I still like to have this UI change.


Concatenation of Tags:

Same situation in Thunderbird 17.0.2 (Ubuntu 12.04). I forget to mention in the current behavior that messages which have both tags are also shown, i.e.:

If I click/activate two tags (tag1 & tag2) in the quick filter toolbar, mails which have (tag1 OR tag2) OR (tag1 AND tag2) are displayed.



[1] https://addons.mozilla.org/en-US/thunderbird/addon/quickfiltertoolbar/
Comment 3 Magnus Melin 2013-02-13 12:43:12 PST
Created attachment 713574 [details] [diff] [review]
proposed fix

I don't see any reason for the current OR-design. Typically you drill down to a particular msg, or messages.
Comment 4 Andrew Sutherland [:asuth] 2013-02-13 13:14:16 PST
This is 100% a change that would need to be run by the current UX lead.  I forget if that's Mike now or not...

I like the idea of adding some type of boolean toggle to the UI so the user can pick what we do.  I think we could just add a toggle/combo-box off to the side that could be workable and fairly easy to add.

In terms of just changing the behaviour from OR to AND, I am of the mind that changing UI behaviour that we've shipped since 3.1 seems particularly inadvisable, especially since the quick filter bar is among the more extensible subsystems in Thunderbird so it's not like it's impossible to address in an extension.  (See https://github.com/asutherland/qfb-pivot for the source to one I made.)

In terms of why/who uses the 'OR' mechanics, without digging into the historical record, I think the idea was that it would aid in triage.  I know I use the 'or' functionality when dealing with bugzilla traffic to show the union of bugs tagged for either immediate action or awareness/follow-up.  This can be especially helpful with threading turned on.  Under an AND-only regime, I would need to effectively create a tag hierarchy, tagging anything I tag with either bug with a common tag.

(Which is not to say I don't see the benefit of AND being available.  Although I would note that in terms of symmetry, Thunderbird's tag colorization is biased in such a way that we really only make you aware of 1 tag on a message, so people might be inclined to tag things with only a single tag.)
Comment 5 Andrew Sutherland [:asuth] 2013-02-13 13:14:48 PST
Comment on attachment 713574 [details] [diff] [review]
proposed fix

(clearing review request since this is a UX decision first and foremost)
Comment 6 Thomas D. (currently busy elsewhere; needinfo?me) 2013-02-13 13:26:06 PST
(In reply to Andrew Sutherland (:asuth) from comment #4)
> I like the idea of adding some type of boolean toggle to the UI so the user
> can pick what we do.  I think we could just add a toggle/combo-box off to
> the side that could be workable and fairly easy to add.

+1!
Comment 7 Magnus Melin 2013-02-16 04:01:33 PST
(In reply to Thomas D. from comment #6)
> (In reply to Andrew Sutherland (:asuth) from comment #4)
> > I like the idea of adding some type of boolean toggle to the UI so the user
> > can pick what we do.  I think we could just add a toggle/combo-box off to
> > the side that could be workable and fairly easy to add.
> 
> +1!

Well it does get a bit confusing for the initial display: You hit the main "tag" button and messages with tags are shown (but no specific tags yet pressed). Then after you press one tag such a toggle button with "All of/Any of" would make sense. Maybe it should be there but disabled until a tag is hit?

I'd still maintain AND should definitely be the default. Everything else in the quickfilter widget is AND. E.g. it's the text and starred and have attachment and tagged "todo". Having pressed tags suddenly be OR is not consistent.
Comment 8 Thomas D. (currently busy elsewhere; needinfo?me) 2013-02-16 05:41:57 PST
(In reply to Magnus Melin from comment #7)
> I'd still maintain AND should definitely be the default. Everything else in
> the quickfilter widget is AND. E.g. it's the text and starred and have
> attachment and tagged "todo". Having pressed tags suddenly be OR is not
> consistent.

+1!
Comment 9 Andrew Sutherland [:asuth] 2013-02-16 08:44:07 PST
(In reply to Magnus Melin from comment #7)
> I'd still maintain AND should definitely be the default. Everything else in
> the quickfilter widget is AND. E.g. it's the text and starred and have
> attachment and tagged "todo". Having pressed tags suddenly be OR is not
> consistent.

The other place we display a parallel series of buttons as an elaboration on a filter is for the text filter, and it operates on an 'OR' basis too.
Comment 10 Magnus Melin 2013-02-16 11:29:39 PST
True but i wouldn't want to change those as you usually wouldn't have the text match more than one (or two) of them.
Comment 11 Magnus Melin 2013-02-16 13:06:33 PST
Created attachment 714833 [details]
screenshot with all/any button

Here's a screenshot with an All/Any button. 

Or perhaps mode should be changeable by context menu only?
Comment 12 Thomas D. (currently busy elsewhere; needinfo?me) 2013-02-16 13:29:08 PST
(In reply to Magnus Melin from comment #11)
> Created attachment 714833 [details]
> screenshot with all/any button

Looks great to me.
Add a tooltip to each of the dropdown options, and we're done.

> Here's a screenshot with an All/Any button. 
> 
> Or perhaps mode should be changeable by context menu only?

I'd think that's not discoverable enough compared to its importance.
Comment 13 Andrew Sutherland [:asuth] 2013-02-16 23:13:17 PST
I like the look of the all of/any of approach!  I agree that the context menu seems too subtle/undiscoverable.  Let's see what Blake and/or Mike think.  (Note: Monday is a holiday in Canada where Blake/Mike are, I believe.)
Comment 14 Dominik 2013-02-17 10:04:27 PST
I also like the "All of/Any of" dropdown solution.
Comment 15 Dominik 2013-02-17 10:07:55 PST
Created attachment 714935 [details]
Design of tag button with inclusion (plus) and exclusion (minus) activated

This is my suggestion for the quite more intuitive way showing the user that a tag *can* be excluded or included: If it is included, the plus sign of the button is pressed down, if excluded, than the minus sign is pressed down.

How do you think about it? I think it would be helpful for the users experience/intuitive using of Thunderbird.
Comment 16 Blake Winton (:bwinton) (:☕️) 2013-02-17 10:40:32 PST
Comment on attachment 714833 [details]
screenshot with all/any button

Yeah, I like the All of/Any of button.  Consider this a ui-r=me.
Comment 17 Blake Winton (:bwinton) (:☕️) 2013-02-17 10:41:49 PST
Comment on attachment 713574 [details] [diff] [review]
proposed fix

I'm going to ui-r- this, since I think the other proposal fulfills both use cases in a more natural way.
Comment 18 Andrew Sutherland [:asuth] 2013-02-17 10:44:39 PST
Comment on attachment 714935 [details]
Design of tag button with inclusion (plus) and exclusion (minus) activated

For ui-review, you want to set the flag to ? and pick the current Thunderbird UX lead which is either :bwinton or :mconley.  Setting it to '+ should only be done by the UX lead once they've reviewed it.

I like the visual style of what you've done.  What would be the interaction mechanism for this?  Click on the plus minus/specifically and it does that, otherwise clicking on the main body cycles through include/exclude/not factored in?

I do think we want to improve the general accessibility state of the quick filter bar, so a possible variation might be to use the combo-box/button hybrid like we use for the smart reply button in the message header that lets you choose reply-to-list/reply-to-all/etc.  The visual style of the plus minus could be maintained with that; it would just be an alternative to the button cycling through 3 states instead of two.
Comment 19 Blake Winton (:bwinton) (:☕️) 2013-02-17 10:46:51 PST
Comment on attachment 714935 [details]
Design of tag button with inclusion (plus) and exclusion (minus) activated

While I appreciate the conciseness of this design, I don't think it solves the and/or problem that this bug is trying to address, and I'm worried the ± buttons would be too small for users to accurately click on.

But thank you, Dominik, for the mockup, and with your permission, I'ld like to post it on the Thunderbird UX Tumblr at http://breakingtheegg.tumblr.com/
Comment 21 Magnus Melin 2013-02-17 10:56:02 PST
Comment on attachment 714935 [details]
Design of tag button with inclusion (plus) and exclusion (minus) activated

Obsoleting as this is not the scope of this bug
Comment 22 Mike Conley (:mconley) - (Needinfo me!) 2013-02-19 06:51:27 PST
I agree that the "All of / Any of" dropdown is the likely solution here.

I might be stating the obvious, but this should probably default to the current behaviour ("Any of").
Comment 23 Magnus Melin 2013-03-03 04:33:25 PST
Created attachment 720396 [details] [diff] [review]
proposed fix, v2

Here's a version with the mode selector widget.
I had the wrong widget type in the mockup screenshot, so it's slightly different, i'll add a new scrrenshot.

I've kept the defaults to "All of". Mike, why did you prefer "Any of"? 
Like I wrote in an earlier comment you typically drill down to one/a few messages. With "Any of" you just get more messages. Not that i think it's a big deal either way now with the mode widget as you can get whichever mode you like one click away.
Comment 24 Magnus Melin 2013-03-03 04:35:37 PST
Created attachment 720397 [details]
screenshot

Updated screenshot
Comment 25 Andrew Sutherland [:asuth] 2013-03-04 03:32:39 PST
(In reply to Magnus Melin from comment #23)
> I've kept the defaults to "All of". Mike, why did you prefer "Any of"? 

I think this comment is pretty clear:

(In reply to Mike Conley (:mconley) from comment #22)
> I might be stating the obvious, but this should probably default to the
> current behaviour ("Any of").

And I'm in agreement on this.  Current behaviour is "any of", so the default without the user ever changing the combo-box should always be "any of".

As is clear from the bug, I think enough people will have a preference one way or the other that we will want the setting to be pretty sticky once the user has set/changed it.  Magnus, do you have thoughts on how best to accomplish it?  Given that the default ends up being set by the state of the XUL widget, after reordering AND/OR or explicitly setting the initial value to the "OR" case, one possibility is to use XUL's "persist" mechanism/attribute to help ensure we don't forget the state when opening new windows, etc.
Comment 26 Andrew Sutherland [:asuth] 2013-03-04 03:33:25 PST
Comment on attachment 720396 [details] [diff] [review]
proposed fix, v2

Review of attachment 720396 [details] [diff] [review]:
-----------------------------------------------------------------

In general, everything looks good, but I would like to do the real review pass once the default gets corrected.

::: mail/base/content/quickFilterBar.js
@@ +517,5 @@
>      return null;
>    },
>  
>    get maybeActiveFilterer() {
> +    if (this.tabmail.currentTabInfo &&

What's the motivation for these guard changes?  It seems like there's still the potential for an exception to throw since '_ext' could be missing.

::: mail/base/modules/quickFilterManager.js
@@ +700,5 @@
>            if (shouldFilter) {
>              term.op = nsMsgSearchOp.Contains;
> +            // AND for the group. Inside the group we also want AND if the
> +            // mode is set to "All of".
> +            term.booleanAnd = firstIncludeClause || (mode == "AND");

nit: use triple === instead of coercing ==

@@ +864,5 @@
>        }
>      }
>  
> +    // -- nuke existing exposed tags, but not the mode selector (which is first)
> +    while (tagbar.lastChild && tagbar.lastChild != tagbar.firstChild)

nit: please use !== instead of !=
Comment 27 Magnus Melin 2013-03-04 12:13:13 PST
(In reply to Andrew Sutherland (:asuth) from comment #26)
> >    get maybeActiveFilterer() {
> > +    if (this.tabmail.currentTabInfo &&
> 
> What's the motivation for these guard changes?  It seems like there's still
> the potential for an exception to throw since '_ext' could be missing.

_ext is guaranteed to exist if there's a tab - http://mxr.mozilla.org/comm-central/source/mail/base/content/tabmail.xml#232

I forget the details, but while debugging and doing things in (as it turned out) wrong places there were some "automatic" quickfilter call that use activeFilterer but the error message wasn't understandable because it threw in the getter.
Comment 28 Magnus Melin 2013-03-13 04:05:13 PDT
Blake, any input on the default mode?
FWIW, using persist to make it sticky works fine.
Comment 29 Blake Winton (:bwinton) (:☕️) 2013-03-24 17:03:46 PDT
Comment on attachment 720396 [details] [diff] [review]
proposed fix, v2

Yeah, this seems mostly there.  There are a few things to fix on the Mac version, though.  (Screenshot here: https://dl.dropbox.com/u/2301433/Screenshots/MultiTags.png )

When selected, I think we should use the same colour and shape as the rectangles surrounding the tags in the header.  I think I would be happy with either the grey rectangles or the current no-rectangle as the unselected view.

I guess that's really about it.  I was thinking that the rectangles were way too large, but that's kind of covered by the "colour and shape"…  :)

(The way you have it on Linux would also be fine, with the coloured text, and the smaller rectangles, if it was too hard on the Mac to get it working like the Tags header.)

ui-r=me with those fixed.

Thanks,
Blake.
Comment 30 Blake Winton (:bwinton) (:☕️) 2013-03-24 17:07:12 PDT
For the (In reply to Magnus Melin from comment #28)
> Blake, any input on the default mode?

I agree with Mike and Andrew.  The current behaviour is "Any of", so that's what people will be expecting.  And making it sticky will let everyone who wants to change it change it once.
Comment 31 Magnus Melin 2013-03-25 13:29:51 PDT
Created attachment 729158 [details] [diff] [review]
proposed fix, v3

Carrying forward ui-r=bwinton.
I'm leavin the mac aesthetics for someone else, as i don't have a mac. This bug isn't really changing how they look, though i guess the buttons may stretch to get a bit taller because of the mode selector. That may be tricky to change though....
Comment 32 Magnus Melin 2013-04-06 03:37:09 PDT
Created attachment 734221 [details] [diff] [review]
proposed fix, v4

Unbitrot
Comment 33 Andrew Sutherland [:asuth] 2013-04-06 20:04:34 PDT
Thanks for un-bitrotting and apologies about the review delay; I've been working flat-out on some gaia e-mail stuff that stretched on for much longer than I was hoping and wanted to give this a serious review.  I should be able to get to this by the end of day Monday.
Comment 34 Magnus Melin 2013-06-05 13:07:12 PDT
Andrew: review ping
Comment 35 Andrew Sutherland [:asuth] 2013-06-06 17:05:49 PDT
Comment on attachment 734221 [details] [diff] [review]
proposed fix, v4

r=asuth. extremely sorry about the delay.

The conditional bi-directional propagation of information in _populateTagBar is a little odd but seems reasonable.  If you have enough memory of the situation you added that for, a comment would be good.  Right now, I would be assuming that the situation is:
// If we don't have a mode, then update our state to agree with what the UI is currently displaying; this will happen on Thunderbird upgrade
Comment 36 Magnus Melin 2013-06-07 11:25:58 PDT
Yes that was the case it was needed for IIRC.

https://hg.mozilla.org/comm-central/rev/7fff0a956f15 -> FIXED
Comment 37 Mark Banner (:standard8) 2013-06-10 09:32:14 PDT
I'm trying to clean up the state of the tree a bit, and found that this had caused perma-orange on Mac (hidden amongst other failures). I've therefore backed it out apart from the L10n changes (to save churn on those that follow trunk):

https://hg.mozilla.org/comm-central/rev/a1c6d81fabb0

Example failures:

https://tbpl.mozilla.org/php/getParsedLog.php?id=23962067&tree=Thunderbird-Trunk#error7

TEST-START | /Users/cltbld/talos-slave/test/build/mozmill/quick-filter-bar/test-filter-logic.js | test_filter_tags
Step Pass: {"function": "controller.click()"}
Step Pass: {"function": "controller.click()"}
Step Pass: {"function": "controller.click()"}
Step Pass: {"function": "Controller.keypress()"}
Step Pass: {"function": "Controller.keypress()"}
The view is showing the following message header and should not be:
  Subject: Fun Ray-Gun Tomorrow
  Date: Tue Feb 01 2000 13:00:00 GMT-0800 (PST)
  Author: "Zig Zig" <zig@zig.invalid>
  Recipients: "Andy Bell" <andy@bell.invalid>
  Read: false   Flagged: false   Killed: false   Junk: false
  Keywords:
  Folder: QuickFilterBarTags  Key: 301
View State:
********* Current View Contents
. Fun Ray-Gun Tomorrow [QuickFilterBarTags,301]
. Lame Pen Yesterday [QuickFilterBarTags,582]
. Funky Sword In a Fortnight [QuickFilterBarTags,876]
********* end view contents
View: [xpconnect wrapped (nsISupports, nsIMsgDBView, nsITreeView)]
  View Type: eShowQuickSearchResults   View Flags: 0
  Sort Type: byDate   Sort Order: ascending
  Search Terms:
    Virtual Folder Terms:
      (none)
    View Terms:
      (none)
    User Terms:
      tag,contains,$label1
      tag,contains,$label2
    Scope (Folders):
      QuickFilterBarTags
Test Failure: view contains header that should not be present! [msgHdr mailbox://nobody@Local%20Folders/QuickFilterBarTags#301]
Comment 38 Magnus Melin 2013-06-10 12:50:42 PDT
Mac only - I can't test that :/

I'd assume there must be something with how the mozmill test changes the mode that doesn't work on mac. Some key not doing what it does on other platforms?

    7.29  /**
    7.30 - * Set the tag filtering mode. Wait for messages after.
    7.31 - */
    7.32 -function toggle_tag_mode() {
    7.33 -  if (mc.e("qfb-boolean-mode").value === "AND")
    7.34 -    mc.keypress(mc.eid("qfb-boolean-mode"), "VK_DOWN", {}); // = move to "OR";
    7.35 -  else if (mc.e("qfb-boolean-mode").value === "OR")
    7.36 -    mc.keypress(mc.eid("qfb-boolean-mode"), "VK_UP", {}); // = move to "AND";
    7.37 -  else
    7.38 -    throw new Error("Unexpected qfb-boolean-mode value:" +
    7.39 -                    mc.e("qfb-boolean-mode").value);
    7.40 -
    7.41 -  mc.keypress(mc.eid("qfb-boolean-mode"), "VK_RETURN", {});
    7.42 -  wait_for_all_messages_to_load(mc);
    7.43 -}
Comment 39 Magnus Melin 2013-06-13 03:53:54 PDT
Modified toggle_tag_mode and made the eventlistener listen to ValueChange instead. 

Try was successful https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=e2c155bc21aa so i've relanded this

https://hg.mozilla.org/comm-central/rev/1769f50bf805 -> FIXED

Note You need to log in before you can comment on or make changes to this bug.