Closed Bug 515650 Opened 15 years ago Closed 15 years ago

[faceted search]: clicking on the "None" value of a facet does nothing

Categories

(Thunderbird :: Search, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0rc1

People

(Reporter: davida, Assigned: asuth)

References

Details

(Whiteboard: [no l10n impact][ready to land])

Attachments

(1 file)

I'm not sure if the facet value should not look clickable, or if it should do something when we click it.
Flags: blocking-thunderbird3?
marking blocking and targeting to rc1.  If we can make it work by b4 that's great.

I think it'd be good to also use some more specific language in the future, e.g. "No Tags"
Flags: blocking-thunderbird3? → blocking-thunderbird3+
Target Milestone: --- → Thunderbird 3.0rc1
It should do things when you click it.  It used to do so.  Constraint support re-write must have broke it.
Component: Mail Window Front End → Search
QA Contact: front-end → search
Whiteboard: [no l10n impact]
Assignee: nobody → bugmail
Status: NEW → ASSIGNED
console sez:

Error: groupValue is null
Source File: chrome://messenger/content/glodaFacetView.js
Line: 234
I have a mostly throw-away patch that tried to treat the None value as a completely special case that did not play well with the other values and was impossible to use as the basis for a gloda query.  During the course of creating this patch I was concluded the following strategy is instead what we want:

The None value is a sufficiently interesting case that attributes may want to be able to query on it.  However, it is also special enough that it should not be treated as one of the other 'in-domain' values and we certainly do not want to make it the responsibility of the attribute to deal with it.

For example, in the tag case, if a message has no tags we don't want the tag provider to have to create a synthetic null tag.  Instead, the attribute definition indicates that the empty list is significant.  So message.tags == [] instead of message.tags == ["null-sentinel-value"].  However, when indexing Gloda will emit a row into the database with a sentinel value so that query.tags(null) returns that message as a result.  It also becomes possible to have the faceting UI show only messages tagged "awesome" or that are not tagged.  (query.tags("awesome", null).)
Whiteboard: [no l10n impact] → [no l10n impact][has in-progress patch, fix strategy]
Whiteboard: [no l10n impact][has in-progress patch, fix strategy] → [no l10n impact][has in-progress patch, fix strategy][will work this on Nov 8]
This was totally the right thing to do.  The semantics work wonderfully and the actual changes to the faceting code were almost non-existent.

The bad news is that I am revving the schema version again, so kiss your gloda databases goodbye once more.

There is some noise in the patch:

- To make the unit test more thorough I needed to make various code supporting Widget in test_query_core.js a bit more robust and a bit more ugly.  (Gloda generally assumes immutable things, so I had to jump through hoops to make the mutable data structure look immutable.)

- logException has had a bug for a while where it prints the stack trace at the time you call it rather than the stack trace of the exception.
Attachment #411152 - Flags: review?(bienvenu)
Whiteboard: [no l10n impact][has in-progress patch, fix strategy][will work this on Nov 8] → [no l10n impact][has patch, needs review bienvenu]
Comment on attachment 411152 [details] [diff] [review]
v1 special empty value support

I read through this and it all looks OK. I need to run with the patch and make sure it fixes the bug, and that tests works, which I'll do in a few minutes.

incomplete comment:
+   * @param [aAttrDef.emptySetIsSignificant=false] Should we
    * @param aAttrDef.subjectNouns A list of object types (NOUNs) that this
Comment on attachment 411152 [details] [diff] [review]
v1 special empty value support

this works better, but it's not fully working. I've seen several instances of clicking None in the tagged category, and saying "must not be tagged" and the search results don't change. I think I saw the same thing with mailing lists. It's not every time, however.

I do see some console warnings from jquery, but they seem to be static warnings.

The xpcshell tests all pass.
Comment on attachment 411152 [details] [diff] [review]
v1 special empty value support

I can't reproduce the issue I thought I saw - perhaps I was confused about what counts I was expecting to see after adding a none constraint.
Attachment #411152 - Flags: review?(bienvenu) → review+
Whiteboard: [no l10n impact][has patch, needs review bienvenu] → [no l10n impact][ready to land]
(In reply to comment #9)
> (From update of attachment 411152 [details] [diff] [review])
> I can't reproduce the issue I thought I saw - perhaps I was confused about what
> counts I was expecting to see after adding a none constraint.

We have some bugs related to deleted tags I think, definitely if the deletion happens during the same session.  Although I doubt that your testing includes deleting tags, it's a possible explanation.
pushed to trunk:
http://hg.mozilla.org/comm-central/rev/f3a99b4c8275

pushed to comm-1.9.1:
http://hg.mozilla.org/releases/comm-1.9.1/rev/d8c84deb69ad
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: