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)
Thunderbird
Search
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)
38.56 KB,
patch
|
Bienvenu
:
review+
|
Details | Diff | Splinter Review |
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?
Comment 1•15 years ago
|
||
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
Assignee | ||
Comment 3•15 years ago
|
||
It should do things when you click it. It used to do so. Constraint support re-write must have broke it.
Updated•15 years ago
|
Component: Mail Window Front End → Search
QA Contact: front-end → search
Updated•15 years ago
|
Whiteboard: [no l10n impact]
Assignee | ||
Updated•15 years ago
|
Assignee: nobody → bugmail
Status: NEW → ASSIGNED
Reporter | ||
Comment 4•15 years ago
|
||
console sez: Error: groupValue is null Source File: chrome://messenger/content/glodaFacetView.js Line: 234
Assignee | ||
Comment 5•15 years ago
|
||
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]
Assignee | ||
Updated•15 years ago
|
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]
Assignee | ||
Comment 6•15 years ago
|
||
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)
Assignee | ||
Updated•15 years ago
|
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 7•15 years ago
|
||
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 8•15 years ago
|
||
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 9•15 years ago
|
||
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+
Updated•15 years ago
|
Whiteboard: [no l10n impact][has patch, needs review bienvenu] → [no l10n impact][ready to land]
Assignee | ||
Comment 10•15 years ago
|
||
(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.
Assignee | ||
Comment 11•15 years ago
|
||
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.
Description
•