Closed Bug 609941 Opened 14 years ago Closed 14 years ago

Tags with the names of Object-provided attributes break the faceted search UI.

Categories

(Thunderbird :: Search, defect)

Product:
Thunderbird
Component:
Search
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
major

Tracking

(thunderbird3.1 .8-fixed)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 3.3a2
Tracking Flags:
Tracking Status
thunderbird3.1 --- .8-fixed

People

(Reporter: mitra_lists, Assigned: asuth)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

Screen shot of failing search
16.44 KB, image/png
Details
v1 hasOwnProperty fix only for faceting
4.16 KB, patch
asuth
: review+
standard8
: approval-thunderbird3.1.8+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-GB; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.13pre) Gecko/20101104 Lanikai/3.1.7pre

If I type anything into the Search bar (including very common terms) it opens the new tab (as usual) and puts up the "Searching" animation but does not find anything.

This has just started happening, and I'm not aware of anything else that could cause it.  I will leave trying to fix it (e.g. by deleting indexes) for a couple of days in case someone wants me to do any tests to see what is damaged. 


Reproducible: Always
Anything in Tools -> Error console ?
Yes ... whenever I shift focus to the main window I get lots of 

Error: [Exception... "Component returned failure code: 0x80550007 [nsIMsgFolder.getStringProperty]"  nsresult: "0x80550007 (<unknown>)"  location: "JS frame :: chrome://messenger/content/folderPane.js :: getSmartFolderName :: line 2441"  data: no]
Source File: chrome://messenger/content/folderPane.js
Line: 2443

Then when I do the search I get ONCE
Error: this.faceter is undefined
Source File: chrome://messenger/content/glodaFacetBindings.xml
Line: 334
Error: this.faceter is undefined
Source File: chrome://messenger/content/glodaFacetBindings.xml
Line: 338
Did this started happening after a update or something ?

(In reply to comment #2)
> 
> Then when I do the search I get ONCE
> Error: this.faceter is undefined
> Source File: chrome://messenger/content/glodaFacetBindings.xml
> Line: 334
> Error: this.faceter is undefined
> Source File: chrome://messenger/content/glodaFacetBindings.xml
> Line: 338

Andrew is this a real issue ?
Hi Ludovic - I'm not sure, I run the nightly builds, and update most days. Its also possible that it was caused by a crash (of the computer, not TB) e.g. running out of battery - which does not shut down gracefully.
The most likely explanation is database corruption that is only detected after the database is in use.  Other possibilities include whatever is making that folderPane/getSmartFolderName happy cascading into causing gloda problems.

Easy enough to decide which it is.  If you delete the global-messages-db.sqlite file and things get fixed, then it was gloda database corruption.
(In reply to comment #5)
> folderPane/getSmartFolderName happy cascading into causing gloda problems.

s/happy/unhappy/
One more strange data point, 

If I search IN a folder (i.e. the inner window search field - I'm not sure what its called) and it finds nothing, then I hit "Enter" again to carry the search to the whole database it works.
Deleteing global-messages-db.sqlite
didn't make search work, but it now generates an error I didn't see before.
Error: groups[valId].push is not a function
Source File: file:///Applications/Lanikai.app/Contents/MacOS/modules/gloda/facet.js
Line: 378

I also notice that Auto-updating isn't functioning, that it says there are no new versions to download - even though I'm on 4nov
(In reply to comment #8)
 
> I also notice that Auto-updating isn't functioning, that it says there are no
> new versions to download - even though I'm on 4nov
Updates are currently broken.
Updating to the 8Nov version doesn't fix it either, even with deleting the global-messages-db.sqlite
(In reply to comment #7)
> One more strange data point, 
> 
> If I search IN a folder (i.e. the inner window search field - I'm not sure what
> its called) and it finds nothing, then I hit "Enter" again to carry the search
> to the whole database it works.

Quick filter bar.

This suggests your virtual folders are screwed up and this throws an exception that manages to break gloda search.

I think bienvenu and protz were tracking something about broken smart folders... cc'ing with the intention that they dupe this or what not...
Are your unified folders (unified inbox, unified archives, etc.) broken as well ?

bug 607981 is the one you had in mind.
Virtual folders  (by this I presume you mean saved searches?) look fine to me.

I suspected the start of this bug might have been coincidental with deleting a folder (after emptying it of messages).

I've gone into the virtual folders and edited each of them to make sure that they aren't trying to search the deleted folder, but that doesn't seem to make a difference i.e. search still doesn't work. 

I do now see this error 
2010-11-09 14:58:31	gloda.ds.qfq	ERROR	Exception: TypeError: document is null
Here's a screen shot of it attempting to search, maybe gives a clue as to what part of the code is broken?
Mitra, can you mail me the virtualFolders.dat file from your user profile dir, along with panacea.dat?
Actually, iirc, that doesn't indicate the smart folders are messed up - it indicates that a local folder doesn't exist at all, and we get an error trying to get the database for the folder to try to get the smart folder name property. And since we now catch this exception, that can't be the direct cause of the issue. I'd suggest turning on gloda logging (asuth can help you there) and see if we can find where things are going south.
I deleted a folder around the time this happened, maybe the delete didn't happen properly (though the folder was still there, in the Trash till yesterday). 

Asuth - can you help me turn on the gloda logging?
https://developer.mozilla.org/en/Thunderbird/Gloda_debugging
https://wiki.mozilla.org/Thunderbird:Using_Gloda

It essentially revolves around turning on some hidden prefs. You also need to make sure that your console is enabled, so that you can see the dump() output.
David: 
Gloda is enabled (2nd link above)
mailnews.database.global.logging.console was already set to true
mailnews.database.global.logging.dump was set to false, but just seems to control where the errors go
mailnews.database.global.logging.net  same applies 

It also says
some of the debug-level log statements and their arguments are entirely predicated on the gloda logging level, but nothing about how to set that level, and there are no obvious preferences for doing so.
(In reply to comment #19)
> mailnews.database.global.logging.dump was set to false, but just seems to
> control where the errors go

This also sets the logging level to everything.  This is the thing to turn on.

Except that is unlikely to tell us much extra that's interesting.  The "document is null" error means something is going horribly off the rails.  It sounds like your JS engine is broken or you have a badly behaved extension.
I'm running Firefox as well - and have no reason to suspect the JS as everything else behaves normally - its not hte kind of thing I muck with.

Extensions running are: Nostalgy; Evernote; Junquilla; ShowInOut; StarMoveContact
I've disabled all of them; restarted and still see the problem.

Is it worth trying to figure out how its broken and why TB is so horribly hosed by something that can only have happened in normal use? e.g. some debugging would be useful to show exactly WHICH folder is hosed so at least I can remove it. 

If not I'd appreciate a suggestion on how to clean up.
So, I guess the "document is null" error could be the result of a query issued in a window context that somehow got destroyed by the time the query completed, but that still wouldn't explain why the facet.js:378 error looks like the JS engine going off the rails...

Please bring up the config editor/about:config (Thunderbird Preferences, Advanced Page, General Tab, Config Editor button), filter on "jit" and set javascript.options.jit.chrome and javascript.options.jit.content to false by double-clicking on them.  Restart and try search again.  I'm somewhat freaked out that both of those are defaulting to true in Thunderbird 3.1.x; that doesn't seem right.
Done - but no difference, 

The odd thing to me is that the search window is not being drawn correctly (see the screen shot I uploaded earlier). 

To track down a prior bug, Daniel sent me a version of TB with some error checking/reporting he'd added - happy to do the same if it would help locate the problem. 

- Mitra
Hm, guess that it changes nothing is good news.  Feel free to turn the preferences back on.  Thank you for checking.

The faceted search interface is just an HTML page, and since we're dying part-way through building it, it makes sense that it will end up in an unhappy state.

Are you still getting folderPane.js errors?  Actually, if you could paste a fresh set of all the errors you see in the error console, that would be very helpful in order to narrow down where instrumentation would need to be added.  Specifically, clear the error console before typing anything in the global search box, then start the search and give it a few seconds.  Then paste what shows up in the error console here (or in an attachment if there's a lot).
OS: Mac OS X → Windows 7
OS: Windows 7 → Mac OS X
Lots of these - including when switching focus
Error: [Exception... "Component returned failure code: 0x80550007 [nsIMsgFolder.getStringProperty]"  nsresult: "0x80550007 (<unknown>)"  location: "JS frame :: chrome://messenger/content/folderPane.js :: getSmartFolderName :: line 2441"  data: no]
Source File: chrome://messenger/content/folderPane.js
Line: 2443

Occasionally this
Error: groups[valId].push is not a function
Source File: file:///Applications/Lanikai.app/Contents/MacOS/modules/gloda/facet.js
Line: 378


I just saw this for the first time as a message and I only get it if the search gets as far as offering me possible email addresses to choose from.
2010-11-15 12:03:10     gloda.ds.qfq     ERROR     Exception: TypeError: groups[valId].push is not a function

And sometimes this - which I think is expected
mitra.biz : server does not support RFC 5746, see CVE-2009-3555

That;s all I'm seeing now, though I was seeing the facet error at a different address. 

-- at startup I get ---
Cannot get rules: [Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getCharPref]"  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: chrome://nostalgy/content/nostalgy.js :: anonymous :: line 95"  data: no]

Error: parentClassOrObject is undefined
Source File: chrome://webclipper3-common/content/libs/evernote/evernote.js
Line: 39

ShowInOut 0.7.12 appName=Thunderbird appVersion=3.1.7pre
The best explanation I can come up with for the facet.js:378 failure is that you have a tag that was created with a very odd name (it need not have that name right now) and accordingly has a very odd key that triggers something buglike in the JS engine.  If you open the prefs.js file in your profile folder and search down to the lines that look like:

user_pref("mailnews.tags.KEYNAME.SOMETHING", "YADDA");

and see something crazy looking, that is likely the problem.


Otherwise, yes, I'm going to need to get you a custom build or a helper debug extension or something, because whatever is happening is really wacky.

note: I did look into the possibility that the folderPane.js anger is related to the gloda faceting failure, but I don't see any candidate paths.  Gloda would not index a folder that (reliably) makes it angry, and so faceting results could not include folders that make it angry.  Additionally, the only facet that should remotely be able to trigger XPCOM explosions to make faceting unhappy would be the synthetic "Account" facet, but that is a singular attribute and so would be faceted by the DiscreteFaceter, not the DiscreteSetFaceter (which is the one that is exploding).
For tags I've got the lines below.

The weird looking one is Jürgen (J<u umlaut>rgen) not sure if that could do it. Tags (including that one) seem to be behaving fine. 

There are also weird looking machine-readable ones, especially anything with a directory path, and user_pref("print.macosx.pagesetup-2"

user_pref("mailnews.tags.au.tag", "AU");
user_pref("mailnews.tags.call.tag", "Call");
user_pref("mailnews.tags.edit.tag", "Edit");
user_pref("mailnews.tags.germany.tag", "Germany");
user_pref("mailnews.tags.india.tag", "India");
user_pref("mailnews.tags.j&apw-rgen.tag", "Jürgen");
user_pref("mailnews.tags.lili.tag", "Lili");
user_pref("mailnews.tags.online.tag", "Online");
user_pref("mailnews.tags.read.tag", "Read");
user_pref("mailnews.tags.trip.tag", "Trip");
user_pref("mailnews.tags.ues.tag", "UES");
user_pref("mailnews.tags.us.tag", "US");
user_pref("mailnews.tags.version", 2);
user_pref("mailnews.tags.watch.tag", "Watch");

The only other odd things I notice comparing to an old version of prefs.js is ...
user_pref("mail.spotlight.lastFolderIndexedUri", "");
which previously always had a folder name in it. 

I also notice that I have deleted one or two tags - I wonder what would happen if a tag was deleted but some messages were still tagged? 

What would happen if the prefs cached a location e.g. for saving a file, and then I deleted that directory (I've moved directories around, but nothing relating to the profile). 

Anything else I can look at for you?
Thanks for checking.  Sadly that all looks above board.  I'll create an extension that can extract the information I need.
(In reply to comment #27)
> user_pref("mailnews.tags.j&apw-rgen.tag", "Jürgen");

I could see that being problematic in a few ways.
Ok - interesting we might be getting closer to the bug

I just created a new tag   AüB
and it turned up in prefs as 
user_pref("mailnews.tags.a&apw-b.tag", "AüB");

i.e. its consistent with the tag for Jürgen

I won't make any changes (like trying to delete that tag) until I hear from you.

- Mitra
The tag keys are created by performing a utf-7 conversion variant.  The downmapping is intentional.  Importantly, the message tag service treats the string as an opaque C string and does not attempt to decode it.  From the perspective of the JavaScript code that is dying, this is all kosher.  I was hoping there was somehow a tag that was valid utf-8 which might get into some XPConnect ambiguity.
Do those odd characters work in both sides of that user_pref? 

If that's fine, then any progress on some further instrumentation to try and track this down. 

- Mitra
Since noone seems interested in tracking this down, Daniel, Andrew, could one of you just give me some suggestions as to what I should delete or whatever to restore the system, since as it stands I can't use Search!
I've done some more debugging on this - added some code to report which folders were causing problems and it turned out to be four ordinary folders, which I've deleted (and emptied Trash) - the folders in all other respects looked intact. 

Having deleted these folders Search still fails to work - with the same problems. 

The only error now showing is: 
Error: this.faceter is undefined
Source File: chrome://messenger/content/glodaFacetBindings.xml
Line: 334
which shows when I mouse-over the three check-boxes (see the screen shot)

I also occasionally - not connected to the Search - see
Error: groups[valId].push is not a function
Source File: file:///Applications/Lanikai.app/Contents/MacOS/modules/gloda/facet.js
Line: 378

and 
2010-12-06 10:27:59	gloda.ds.qfq	ERROR	Exception: TypeError: groups[valId].push is not a function

Any hints as to how to track this down...
Since you are willing to get your hands dirty with the code, this simplifies things.  (I've been thinking of creating a debug extension but there's a lot of moving parts for that and trial and error with try builds is troublesome.)

You would want to wrap the call to faceter.facetItems in the FacetDriver._drive function at facet.js:132 in a try/catch loop and dump that faceter that was active.

Something like:
try {
  faceter.facetItems(this.items);
}
catch(ex) {
  dump("Exception: ex\n");
  dump(JSON.stringify(faceter, null, 2) + "\n");
}

The intent is to figure out what facet/attribute is breaking things.
Ok - now seeing several errors: 
Error: row is undefined; Source File: chrome://gloda/content/glodacomplete.xml; Line: 88 
Appears about 5 times, then

Error: too much recursion
Source File: file:///Applications/Lanikai.app/Contents/MacOS/modules/gloda/facet.js Line: 138
appears once followed by ...

Error: this.faceter is undefined
Source File: chrome://messenger/content/glodaFacetBindings.xml
Line: 338
appearing twice.

Note that 138 is the line with the dump(JSON...)
I'm assuming you deliberately have the "ex" inside the string, ratehr than outside? 

Where should I look for the dump?

- Mitra
Ah, I was hoping the data structure would not be self-recursive.  That is making the JSON call unhappy.  Can you try replacing JSON.stringify(blah, null, 2) with uneval(blah)?  That should do recursion detection; I always forget it exists.
The dump will show up on the terminal output.  You may need to run with -console on windows or from the command line on OS X or linux.  You will also need the preference "browser.dom.window.dump.enabled" set to true unless you are running a debug build.
You need to start from Thunderbird from terminall.app to get the dumps.
Ok - I did the following

Changed the debug code so it now reads
try {
  faceter.facetItems(this.items);
}
catch(ex) {
  dump("Exception: ex\n");
  dump(uneval(faceter));
}


There is no preference remotely like  browser.dom.window.dump.enabled

cd /Applications/Lanikai.app/Contents/MacOS ; ./thunderbird.bin

Very weirdly - the search now works - I guess catching the exception stopped it propogating up the code in some way. though it reports the dump below ...

I've checked - by returning code to just the "faceter.facetItems" and this is not an artifact of running from the command line.

- Mitra

Exception: ex
({attrDef:#4={provider:#5={providerName:"gloda.explattr", strings:{_url:"chrome://messenger/locale/gloda.properties", get _stringBundle () stringBundle}, _log:{_name:"gloda.explattr", _appenders:[], _repository:#1={_rootLogger:#3={_name:"root", _appenders:[], _repository:#1#, _level:20}}, parent:{_name:"gloda", _appenders:[{_name:"ConsoleAppender", _formatter:#2={_dateFormat:"%Y-%m-%d %H:%M:%S"}, _level:50}, {_name:"DumpAppender", _formatter:#2#, _level:0}], _repository:#1#, parent:#3#, _level:0}}, _msgTagService:({}), init:(function gloda_explattr_init() {this._log = Log4Moz.repository.getLogger("gloda.explattr");this._msgTagService = Cc['@mozilla.org/messenger/tagservice;1'].getService(Ci.nsIMsgTagService);try {this.defineAttributes();} catch (ex) {this._log.error("Error in init: " + ex);throw ex;}}), NOTABILITY_STARRED:16, NOTABILITY_TAGGED_FIRST:8, NOTABILITY_TAGGED_ADDL:1, defineAttributes:(function () {this._attrTag = Gloda.defineAttribute({provider: this, extensionName: Gloda.BUILT_IN, attributeType: Gloda.kAttrExplicit, attributeName: "tag", bindName: "tags", singular: false, emptySetIsSignificant: true, facet: true, subjectNouns: [Gloda.NOUN_MESSAGE], objectNoun: Gloda.NOUN_TAG, parameterNoun: null, propertyChanges: ["keywords"]});this._attrStar = Gloda.defineAttribute({provider: this, extensionName: Gloda.BUILT_IN, attributeType: Gloda.kAttrExplicit, attributeName: "star", bindName: "starred", singular: true, facet: true, subjectNouns: [Gloda.NOUN_MESSAGE], objectNoun: Gloda.NOUN_BOOLEAN, parameterNoun: null});this._attrRead = Gloda.defineAttribute({provider: this, extensionName: Gloda.BUILT_IN, attributeType: Gloda.kAttrExplicit, attributeName: "read", singular: true, subjectNouns: [Gloda.NOUN_MESSAGE], objectNoun: Gloda.NOUN_BOOLEAN, parameterNoun: null});this._attrRepliedTo = Gloda.defineAttribute({provider: this, extensionName: Gloda.BUILT_IN, attributeType: Gloda.kAttrExplicit, attributeName: "repliedTo", singular: true, subjectNouns: [Gloda.NOUN_MESSAGE], objectNoun: Gloda.NOUN_BOOLEAN, parameterNoun: null});this._attrForwarded = Gloda.defineAttribute({provider: this, extensionName: Gloda.BUILT_IN, attributeType: Gloda.kAttrExplicit, attributeName: "forwarded", singular: true, subjectNouns: [Gloda.NOUN_MESSAGE], objectNoun: Gloda.NOUN_BOOLEAN, parameterNoun: null});}), process:(function Gloda_explattr_process(aGlodaMessage, aRawReps, aIsNew, aCallbackHandle) {var aMsgHdr = aRawReps.header;aGlodaMessage.starred = aMsgHdr.isFlagged;if (aGlodaMessage.starred) {aGlodaMessage.notability += this.NOTABILITY_STARRED;}aGlodaMessage.read = aMsgHdr.isRead;var flags = aMsgHdr.flags;aGlodaMessage.repliedTo = Boolean(flags & nsMsgMessageFlags_Replied);aGlodaMessage.forwarded = Boolean(flags & nsMsgMessageFlags_Forwarded);var tags = aGlodaMessage.tags = [];var keywords = aMsgHdr.getStringProperty("keywords");var keywordList = keywords.split(" ");var keywordMap = {};for (let iKeyword = 0; iKeyword < keywordList.length; iKeyword++) {let keyword = keywordList[iKeyword];keywordMap[keyword] = true;}var tagArray = TagNoun.getAllTags();for (let iTag = 0; iTag < tagArray.length; iTag++) {let tag = tagArray[iTag];if (tag.key in keywordMap) {tags.push(tag);}}if (tags.length) {aGlodaMessage.notability += this.NOTABILITY_TAGGED_FIRST + (tags.length - 1) * this.NOTABILITY_TAGGED_ADDL;}yield Gloda.kWorkDone;}), score:(function Gloda_explattr_score(aMessage, aContext) {var score = 0;if (aMessage.starred) {score += this.NOTABILITY_STARRED;}if (aMessage.tags.length) {score += this.NOTABILITY_TAGGED_FIRST + (aMessage.tags.length - 1) * this.NOTABILITY_TAGGED_ADDL;}return score;}), _attrTag:#4#, _attrStar:#6={provider:#5#, extensionName:"built-in", attributeType:3, attributeName:"star", bindName:"starred", singular:true, facet:{type:"default", groupIdAttr:"id", groupComparator:#7=(function gloda_bool_comparator(a, b) {if (a == null) {if (b == null) {return 0;} else {return 1;}} else if (b == null) {return -1;}return b - a;}), filter:null, strings:{facetNameLabel:"Starred"}}, subjectNouns:[102], objectNoun:1, parameterNoun:null, emptySetIsSignificant:false, dbDef:{_id:55, _compoundName:"built-in:star", _attrType:3, _pluginName:"built-in", _attrName:"star", attrDef:#6#, _parameterBindings:{}}, id:55, boundName:"starred", objectNounDef:#10={name:"bool", clazz:#8=function Boolean() {[native code]}, allowsArbitraryAttrs:false, isPrimitive:true, comparator:#7#, toParamAndValue:(function (aBool) {return [null, aBool ? 1 : 0];}), id:1, class:#8#, idAttr:"id", attribsByBoundName:{}, domExposeAttribsByBoundName:{}, objectNounOfAttributes:[#6#, #9={provider:#5#, extensionName:"built-in", attributeType:3, attributeName:"read", singular:true, subjectNouns:[102], objectNoun:1, parameterNoun:null, emptySetIsSignificant:false, dbDef:{_id:56, _compoundName:"built-in:read", _attrType:3, _pluginName:"built-in", _attrName:"read", attrDef:#9#, _parameterBindings:{}}, id:56, boundName:"read", objectNounDef:#10#, facet:null}, #11={provider:#5#, extensionName:"built-in", attributeType:3, attributeName:"repliedTo", singular:true, subjectNouns:[102], objectNoun:1, parameterNoun:null, emptySetIsSignificant:false, dbDef:{_id:57, _compoundName:"built-in:repliedTo", _attrType:3, _pluginName:"built-in", _attrName:"repliedTo", attrDef:#11#, _parameterBindings:{}}, id:57, boundName:"repliedTo", objectNounDef:#10#, facet:null}, #12={provider:#5#, extensionName:"built-in", attributeType:3, attributeName:"forwarded", singular:true, subjectNouns:[102], objectNoun:1, parameterNoun:null, emptySetIsSignificant:false, dbDef:{_id:58, _compoundName:"built-in:forwarded", _attrType:3, _pluginName:"built-in", _attrName:"forwarded", attrDef:#12#, _parameterBindings:{}}, id:58, boundName:"forwarded", objectNounDef:#10#, facet:null}], actions:[]}}, _attrRead:#9#, _attrRepliedTo:#11#, _attrForwarded:#12#}, extensionName:"built-in", attributeType:3, attributeName:"tag", bindName:"tags", singular:false, emptySetIsSignificant:true, facet:#30={type:"default", groupIdAttr:"key", groupComparator:#28=(function gloda_noun_tag_comparator(a, b) {if (a == null) {if (b == null) {return 0;} else {return 1;}} else if (b == null) {return -1;}return a.tag.localeCompare(b.tag);}), filter:null, strings:{facetNameLabel:"Tags", mustMatchLabel:"must be tagged #1", cantMatchLabel:"can't be tagged #1", mustMatchNoneLabel:"can't be tagged", mustMatchSomeLabel:"must be tagged"}}, subjectNouns:[102], objectNoun:50, parameterNoun:null, propertyChanges:["keywords"], dbDef:{_id:54, _compoundName:"built-in:tag", _attrType:3, _pluginName:"built-in", _attrName:"tag", attrDef:#4#, _parameterBindings:{online:67, lili:68, us:69, ues:70, 'j&apw-rgen':71, au:72, call:73, india:74, germany:75, edit:76, read:77, trip:78}}, id:54, boundName:"tags", objectNounDef:{name:"tag", clazz:#29=({}), usesParameter:true, allowsArbitraryAttrs:false, idAttr:"key", _msgTagService:({}), _tagMap:{'a&apw-b':#13=({}), au:#14=({}), call:#15=({}), edit:#16=({}), germany:#17=({}), india:#18=({}), 'j&apw-rgen':#19=({}), lili:#20=({}), online:#21=({}), read:#22=({}), tamera:#23=({}), trip:#24=({}), ues:#25=({}), us:#26=({}), watch:#27=({})}, _tagList:[({}), ({}), ({}), ({}), ({}), ({}), ({}), ({}), ({}), ({}), ({}), ({}), ({}), ({}), ({})], _init:(function () {this._msgTagService = Cc['@mozilla.org/messenger/tagservice;1'].getService(Ci.nsIMsgTagService);this._updateTagMap();}), getAllTags:(function gloda_noun_tag_getAllTags() {if (this._tagList == null) {this._updateTagMap();}return this._tagList;}), _updateTagMap:(function gloda_noun_tag_updateTagMap() {this._tagMap = {};var tagArray = this._tagList = this._msgTagService.getAllTags({});for (let iTag = 0; iTag < tagArray.length; iTag++) {let tag = tagArray[iTag];this._tagMap[tag.key] = tag;}}), comparator:#28#, userVisibleString:(function gloda_noun_tag_userVisibleString(aTag) {return aTag.tag;}), toParamAndValue:(function gloda_noun_tag_toParamAndValue(aTag) {return [aTag.key, null];}), toJSON:(function gloda_noun_tag_toJSON(aTag) {return aTag.key;}), fromJSON:(function gloda_noun_tag_fromJSON(aTagKey, aIgnored) {var tag = this._tagMap[aTagKey];if (tag === undefined && this._msgTagService.isValidKey(aTagKey)) {this._updateTagMap();tag = this._tagMap[aTagKey];}return tag;}), getTag:(function gloda_noun_tag_getTag(aTagKey) {return this.fromJSON(aTagKey);}), id:50, class:#29#, attribsByBoundName:{}, domExposeAttribsByBoundName:{}, objectNounOfAttributes:[#4#], actions:[]}}, facetDef:#30#, groups:{}, groupMap:{}, groupCount:0})
Whoops.  It's the tag named "watch".  We need to be initializing the objects we are using as dictionaries using {__proto__: null} to stop this kind of thing from happening.  Oh Python, how I miss thee and thine explicit set/dictionary objects.
Assignee: nobody → bugmail
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Summary: Gloda search completed stopped finding anything → Tags with the names of Object-provided attributes break the faceted search UI.
(In reply to comment #40)
> Very weirdly - the search now works - I guess catching the exception stopped it
> propogating up the code in some way. though it reports the dump below ...

Yeah, unless we re-throw the exception, it isolates the damage.  And I was just being dumb about putting the "ex" in the string; was a bit rushed in my composition.

In any event, we can now fix this bug.  Thank you so much for your persistence and excellent diagnostic support!
This rather aggressively changes {} to {__proto__: null} pretty much everywhere we are using JS objects as maps/dictionaries in gloda.  Specifically, I manually checked all the gloda module sources and glodaFacetView.js for {}.

Sid, I'm asking you for review mainly to sanity check my belief that this is an inherently safe thing to do and that there will be no horrible gotchas from doing so.  I am not requesting/expecting that you perform your own code walking.

Case analysis of potential collisions breaks out like this:

a) There was never any danger of collision because the key-space is all integer values that get stringified and integer values will never overlap with anything in Object.prototype.

b) Collisions would not occur for legal values, but could occur for illegal values that could be provided to us.  For example, message-id's.

c) The key-space was entirely user specified, like tag values, and so collisions were quite possible.

Of course, only things that use "in" to check for membership could be tricked by the problem since our enumeration would not walk up into the prototype ever when using Iterator(), and only for expando things on Object.prototype when not using Iterator (which should ideally never happen).
Attachment #496008 - Flags: review?(sid.bugzilla)
Thanks Andrew 

One thing this bug also shows up is the poor level of error reporting, (something I've seen commonly on OpenSource projects where I *was* coding)

 e.g. the getSmartFolderName should have reported which folder it was having problems with, and the faceter should probably have reported what part of what structure it couldn't handle.
Yes, it's rare to have something show up in the error console that is useful.

Thanks to major improvements in the performance of the JS engine we can likely start to be a bit more free about logging and tie it into the exception handling mechanism.  (For Thunderbird 3.0, profiling of the gloda indexer which was some of the first logging-enabled code indicated a non-trivial cost which led to more aggressive thresholding.  This definitely did not encourage sprinkling logging code around the system.)
I've removed all the "watch" tags from message and deleted the tag, but I'm still getting errors (different ones now), specifically:

Error: a.tag is undefined
Source File: file:///Applications/Lanikai.app/Contents/MacOS/modules/gloda/noun_tag.js
Line: 91

Is this related?

Is there a way to dump a backtrace, so that I can see what this is being called from?
So the main reason I'm concerned about this patch is that since toString isn't in the prototype any more, printing out any such dictionary will throw an exception. I'm worried that even though toString with a dictionary is not particularly useful, an exception just because of a dump(dict) is not good.

I've started a discussion on tb-planning about this.
Comment on attachment 496008 [details] [diff] [review]
replace {} with {__proto__: null} basically everywhere gloda creates JS objects for use as dictionaries v1

Per discussion on IRC, we're switching just the faceting stuff to hasOwnProperty instead.
Attachment #496008 - Flags: review?(sid.bugzilla) → review-
Per IRC, this just fixes the affected faceting logic and uses hasOwnProperty to perform the checks.  from IRC/carrying forward r=sid0.

This works with my litmus test of performing a gloda search query where some of the results include a tag that was created with the name "watch".
Attachment #496008 - Attachment is obsolete: true
Attachment #496940 - Flags: review+
pushed to trunk:
http://hg.mozilla.org/comm-central/rev/26c975e35f4c
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Attachment #496940 - Flags: approval-thunderbird3.1.8?
(In reply to comment #49)
 
> This works with my litmus test of performing a gloda search query where some of

Did you meant mozmill ? or Do want some test like this to be added to litmus ?
(In reply to comment #51)
> (In reply to comment #49)
> > This works with my litmus test of performing a gloda search query where some of
> Did you meant mozmill ? or Do want some test like this to be added to litmus ?

Sorry, I was using litmus as in the expression as opposed to the tool (name) based on the expression.

I don't think this specific change merits a litmus-the-tool test because it's exceedingly unlikely to regress unless major changes happen to the faceting logic.  (And major changes to the faceting logic would be accompanied by proper unit tests.)
Target Milestone: --- → Thunderbird 3.3a2
Could someone clarify "pushed to trunk" - I'm not sure if this patch made it into the nighties yet?

Is there a way to tell when a fix gets there (in order to test)

My current info string reads ...
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.14pre) Gecko/20101211 Lanikai/3.1.8pre
(In reply to comment #53)
> Could someone clarify "pushed to trunk" - I'm not sure if this patch made it
> into the nighties yet?
> 
> Is there a way to tell when a fix gets there (in order to test)

It's coming. It should be in today's nightly.
(In reply to comment #54)
> (In reply to comment #53)
> > Could someone clarify "pushed to trunk" - I'm not sure if this patch made it
> > into the nighties yet?
> > 
> > Is there a way to tell when a fix gets there (in order to test)
> 
> It's coming. It should be in today's nightly.

Except Mitra is on the 3.1.x branch, so it won't be in today's nightly. I'll hopefully get chance to look at the approval tomorrow.
Thanks - I'm not sure why I'm on the 3.1.x branch except that neither 
http://www.mozilla.org/developer/#builds
nor anything in
http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/
gives any indication of which build is which, or which is most useful to be testing (and reasonably stable enough to risk live email on - which is how I think the testing is useful).
Mark - is the bug fix in yet, just asking because I'm still seeing the bug in Comment #46 and wondering if that is a different bug that needs reporting. 

I'm also seeing faceter errors

Error: this.faceter is undefined
Source File: chrome://messenger/content/glodaFacetBindings.xml
Line: 334
and also at Line: 338

and wondering if that is the same problem or not

Note that i no longer have any of the "Watch" tags and searches mostly work,
(In reply to comment #46)
> I've removed all the "watch" tags from message and deleted the tag, but I'm
> still getting errors (different ones now), specifically:
> 
> Error: a.tag is undefined
> Source File:
> file:///Applications/Lanikai.app/Contents/MacOS/modules/gloda/noun_tag.js
> Line: 91
> 
> Is this related?

Oops.  Missed this before.  This could be XPCOM getting angry about how the string is defined versus how it is actually encoded.  I will check.
 
> Is there a way to dump a backtrace, so that I can see what this is being called
> from?

In a debug build, you can use "debugger;" to cause that to happen.  But you probably don't have a debug build, so you can do something like this:
  try {throw new Error("stack!");} catch (ex) {dump(ex.stack);}

This is likely just the sorting logic for the faceting UI trying to alphabetically sort your tags once faceting completes.
(In reply to comment #57)
> Mark - is the bug fix in yet, just asking because I'm still seeing the bug in
> Comment #46 and wondering if that is a different bug that needs reporting. 
> 
> I'm also seeing faceter errors
> 
> Error: this.faceter is undefined
> Source File: chrome://messenger/content/glodaFacetBindings.xml
> Line: 334
> and also at Line: 338
> 
> and wondering if that is the same problem or not

This is the hovering logic firing on the checkboxes in a faceted search UI that is either not yet populated because the query is still pending or because things broke for other reasons.  I think we have an existing bug on this.
Re Comment #58 - here's the trace - not sure where to stick more debugging? 

gloda_noun_tag_comparator(watch,[object XPCWrappedNative_NoHelper])@file:///Applications/Lanikai.app/Contents/MacOS/modules/gloda/noun_tag.js:92
comparatorHelper([object Array],[object Array])@file:///Applications/Lanikai.app/Contents/MacOS/modules/gloda/facet.js:392
([object Array])@file:///Applications/Lanikai.app/Contents/MacOS/modules/gloda/facet.js:394
([object Array])@file:///Applications/Lanikai.app/Contents/MacOS/modules/gloda/facet.js:307
()@file:///Applications/Lanikai.app/Contents/MacOS/modules/gloda/facet.js:132
FacetDriver_go([object Array],(function () {this.planLayout();var uiFacets = document.getElementById("facets");if (!this.everFaceted) {this.everFaceted = true;this.faceters.sort(this._groupCountComparator);for each (let [, faceter] in Iterator(this.faceters)) {let attrName = faceter.attrDef.attributeName;let explicitBinding = document.getElementById("facet-" + attrName);if (explicitBinding) {explicitBinding.explicit = true;explicitBinding.faceter = faceter;explicitBinding.attrDef = faceter.attrDef;explicitBinding.facetDef = faceter.facetDef;explicitBinding.nounDef = faceter.attrDef.objectNounDef;explicitBinding.orderedGroups = faceter.orderedGroups;if (faceter.groupCount >= 1 || explicitBinding.getAttribute("type").indexOf("boolean") != -1) {try {explicitBinding.build(true);} catch (e) {logObject(explicitBinding);logException(e);}explicitBinding.removeAttribute("uninitialized");}faceter.xblNode = explicitBinding;continue;}if (faceter.groupCount <= 1) {faceter.xblNode = null;continue;}faceter.xblNode = uiFacets.addFacet(faceter.type, faceter.attrDef, {faceter: faceter, facetDef: faceter.facetDef, orderedGroups: faceter.orderedGroups, maxDisplayRows: this.maxDisplayRows, explicit: false});}} else {for each (let [, faceter] in Iterator(this.faceters)) {if (!faceter.xblNode || faceter.constraint && !faceter.xblNode.canUpdate) {continue;}if (faceter.groupCount <= 1 && !faceter.constraint && (!faceter.xblNode.explicit || faceter.type == "date")) {$(faceter.xblNode).hide();} else {faceter.xblNode.orderedGroups = faceter.orderedGroups;faceter.xblNode.build(false);$(faceter.xblNode).show();}}}if (!this._timelineShown) {this._hideTimeline(true);}this._showResults();if (this._callbackOnFacetComplete) {let callback = this._callbackOnFacetComplete;this._callbackOnFacetComplete = null;callback();}}),[object Object])@file:///Applications/Lanikai.app/Contents/MacOS/modules/gloda/facet.js:117
([object Array])@chrome://messenger/content/glodaFacetView.js:453
()@chrome://messenger/content/glodaFacetView.js:416
reachOutAndTouchFrame()@chrome://messenger/content/glodaFacetView.js:913
onload([object Event])@chrome://messenger/content/glodaFacetView.xhtml:1
gloda_noun_tag_comparator(watch,[object XPCWrappedNative_NoHelper])@file:///Applications/Lanikai.app/Contents/MacOS/modules/gloda/noun_tag.js:92
comparatorHelper([object Array],[object Array])@file:///Applications/Lanikai.app/Contents/MacOS/modules/gloda/facet.js:392
([object Array])@file:///Applications/Lanikai.app/Contents/MacOS/modules/gloda/facet.js:394
([object Array])@file:///Applications/Lanikai.app/Contents/MacOS/modules/gloda/facet.js:307
()@file:///Applications/Lanikai.app/Contents/MacOS/modules/gloda/facet.js:132
FacetDriver_go([object Array],(function () {this.planLayout();var uiFacets = document.getElementById("facets");if (!this.everFaceted) {this.everFaceted = true;this.faceters.sort(this._groupCountComparator);for each (let [, faceter] in Iterator(this.faceters)) {let attrName = faceter.attrDef.attributeName;let explicitBinding = document.getElementById("facet-" + attrName);if (explicitBinding) {explicitBinding.explicit = true;explicitBinding.faceter = faceter;explicitBinding.attrDef = faceter.attrDef;explicitBinding.facetDef = faceter.facetDef;explicitBinding.nounDef = faceter.attrDef.objectNounDef;explicitBinding.orderedGroups = faceter.orderedGroups;if (faceter.groupCount >= 1 || explicitBinding.getAttribute("type").indexOf("boolean") != -1) {try {explicitBinding.build(true);} catch (e) {logObject(explicitBinding);logException(e);}explicitBinding.removeAttribute("uninitialized");}faceter.xblNode = explicitBinding;continue;}if (faceter.groupCount <= 1) {faceter.xblNode = null;continue;}faceter.xblNode = uiFacets.addFacet(faceter.type, faceter.attrDef, {faceter: faceter, facetDef: faceter.facetDef, orderedGroups: faceter.orderedGroups, maxDisplayRows: this.maxDisplayRows, explicit: false});}} else {for each (let [, faceter] in Iterator(this.faceters)) {if (!faceter.xblNode || faceter.constraint && !faceter.xblNode.canUpdate) {continue;}if (faceter.groupCount <= 1 && !faceter.constraint && (!faceter.xblNode.explicit || faceter.type == "date")) {$(faceter.xblNode).hide();} else {faceter.xblNode.orderedGroups = faceter.orderedGroups;faceter.xblNode.build(false);$(faceter.xblNode).show();}}}if (!this._timelineShown) {this._hideTimeline(true);}this._showResults();if (this._callbackOnFacetComplete) {let callback = this._callbackOnFacetComplete;this._callbackOnFacetComplete = null;callback();}}),[object Object])@file:///Applications/Lanikai.app/Contents/MacOS/modules/gloda/facet.js:117
([object Array])@chrome://messenger/content/glodaFacetView.js:453
()@chrome://messenger/content/glodaFacetView.js:416
reachOutAndTouchFrame()@chrome://messenger/content/glodaFacetView.js:913
onload([object Event])@chrome://messenger/content/glodaFacetView.xhtml:1
(In reply to comment #58)
> > Error: a.tag is undefined
> > Source File:
> > file:///Applications/Lanikai.app/Contents/MacOS/modules/gloda/noun_tag.js
> > Line: 91
> > 
> > Is this related?
> 
> Oops.  Missed this before.  This could be XPCOM getting angry about how the
> string is defined versus how it is actually encoded.  I will check.

Yeah, it wasn't that.  Not that I expected it to be that, but I thought had already protected against what is happening.  Turns out I have trouble distinguishing things I imagined from real things, which explains why I can never find where I parked my rocket car.

This is fallout from your removal; I've created bug 619909 and cc'd you to track the process of fixing which I will do very shortly.  A somewhat related bug is bug 522222, but that's more about UI fallout that is fixed when you restart.
(In reply to comment #60)
> Re Comment #58 - here's the trace - not sure where to stick more debugging? 

Thanks for the stack.  This confirms what I thought.
Attachment #496940 - Flags: approval-thunderbird3.1.8? → approval-thunderbird3.1.8+
Blocks: 620780
pushed to comm-1.9.2:
http://hg.mozilla.org/releases/comm-1.9.2/rev/abd5e2fcfb58

Note that I also found a bug that I had not noticed before during testing, bug 620780, that I will follow up on and also try and get landed on 3.1.x.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: