I'm not sure if the facet value should not look clickable, or if it should do something when we click it.
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"
It should do things when you click it. It used to do so. Constraint support re-write must have broke it.
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).)
Created attachment 411152 [details] [diff] [review] v1 special empty value support 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.
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.
(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