Closed Bug 516912 Opened 11 years ago Closed 11 years ago

faceted search/message/any tabs broken by bug 516237

Categories

(Thunderbird :: Search, defect, P1)

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.1a1

People

(Reporter: thomas8, Assigned: Bienvenu)

References

Details

(Whiteboard: [no l10n impact])

Attachments

(3 files, 1 obsolete file)

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20090915 Shredder/3.1a1pre

I am seeing a major regression over yesterday's build (20090914):

- safe-mode, profile-manager, my tb3-only fresh test profile (using since couple of days) with messages in inbox
- at startup, everything looks normal (see messages in inbox)
- do subject or from search - fine
- do "search everything" search --> broken:
-> empties the message list (with no return to ever see messages again, whatever folder you chose, even after clearing the filter)
-> no new tab is opened, no facets seen, just blank message list
-> need to restart TB

Am I the only one who sees this on WinXP? Any help/advice/explanation?
Severity: normal → major
Version: unspecified → Trunk
This WFM on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.4pre) Gecko/20090915 Lightning/1.0pre Shredder/3.0b4pre

Please can you try a 3.0 build from:

ftp://ftp.mozilla.org/pub/thunderbird/nightly/latest-comm-1.9.1/

in case there is something broken in the 3.1 builds.
I'm seeing this also with trunk build on Win7. With 3.0 build all is okay.

So it's a 3.1 problem.
confirming per comment #2. It's trunk-only for me, too, current 3.0b4 is ok.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Faceted search is working for me with:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; de; rv:1.9.3a1pre) Gecko/20090915 Thunderbird/3.1a1pre
Its NOT working with:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; de; rv:1.9.3a1pre) Gecko/20090917 Thunderbird/3.1a1pre
I see nothing in the error console.

Note: This are my own builds, because of the lack of official Mac OS X 3.1a1pre builds at the moment.
It's also working with:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; de; rv:1.9.1.4pre) Gecko/20090917 Shredder/3.0pre
So I can confirm that this is trunk-only.
Just to say that we've seen this bug. At the moment the 3.1 builds are not high priority but we are just keeping builds going so that we can detect these sorts of bugs (which we are grateful for you filing).

I'll be building 3.1 later to fix the Mac build issue, if I see anything later I'll take a look, but I expect it will be complicated.
Only for the record. Lightning stopped to work also with the 0915 version. After clicking the Calendar or Tasks button the message list blanks. With 0914 Lightning works.
(In reply to comment #6)
> Only for the record. Lightning stopped to work also with the 0915 version.
> After clicking the Calendar or Tasks button the message list blanks. With 0914
> Lightning works.

Lightning isn't supported on trunk builds. If you're trying to use Lightning with Thunderbird 3.1 builds, expect it not to work.
OS: Windows XP → All
Hardware: x86 → All
Summary: [faceted search] doesn't work, broken - empties message list (blank), no new tab, no facets, no return -> restart needed → [TB3.1] faceted search/message tabs broken
I've now done some investigations on this bug. It appears an assumption made in folderDisplay.js no longer holds true, and that breaks the FakeTreeBoxObject which seems to break anything to do with messages/folders/search and tabs.

On the console I'm getting:

WARNING: NS_ENSURE_TRUE(mTree == aTree) failed: file /Users/moztest/comm/trunk/src/mozilla/layout/xul/base/src/tree/src/nsTreeSelection.cpp, line 303
-- Exception object --
+ QueryInterface (function) 3 lines
+ message (string) 'Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsITreeSelection.tree]'
+ result (number) 2147549183
+ name (string) 'NS_ERROR_UNEXPECTED'
+ filename (string) 'chrome://messenger/content/folderDisplay.js'
+ lineNumber (number) 1574
+ columnNumber (number) 0
+ location (object) JS frame :: chrome://messenger/content/folderDisplay.js :: FolderDisplayWidget_hookUpFakeTreeBox :: line 1574
+ inner (object) null
+ data (object) null
+ initialize (function) 3 lines

Which points to:

http://hg.mozilla.org/comm-central/annotate/d0e6ed7c9ca1/mail/base/content/folderDisplay.js#l1574

and then:

http://hg.mozilla.org/comm-central/annotate/d0e6ed7c9ca1/mail/base/content/folderDisplay.js#l2274

This makes trunk builds pretty unusable. I'm guessing this is why our mozmill tests are failing there, though I'm also seeing js assertions as well on debug builds :-(
Since we want to stop doing the multiplexed tab thing anyways, I wouldn't spend too much time on this... the effort would probably be better spent just moving us to multiplexed tabs.  Having said that, a quick look at the diffs for the XUL tree widget since 1.9.1 will likely track the cause down.
(In reply to comment #8)
> This makes trunk builds pretty unusable.

Thats not nice, because at the moment I only use trunk builds. Is it known which checkin this does cause?
This has to do with this changeset: http://hg.mozilla.org/mozilla-central/rev/b501121884dd
If I back this out, than faceted search and message tabs are working again in 3.1a1pre.
The message and folder panes for today's (20090926) build is completely broken. When Thunderbird is started, both panes have gone completely blank. 

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.3a1pre) Gecko/20090926 Shredder/3.1a1pre ID:20090926031501
So what is aTree when SetTree is called? Apparently it doesn't implement
nsIBoxObject, and that means that before bug 516237 calling treeselection.single crashes (null pointer).
Attached patch untested hack (obsolete) — Splinter Review
Or does this help? This is pretty much the only thing I can do here.
If this doesn't help, the mailnews code needs to be changed, I think.
(In reply to comment #13)
> So what is aTree when SetTree is called? Apparently it doesn't implement
> nsIBoxObject, and that means that before bug 516237 calling
> treeselection.single crashes (null pointer).

See the links in comment 8. It certainly doesn't crash on the 1.9.1 branch so
there must have been some other change (and afaik it didn't crash on
mozilla-central before bug 516237 landed).
Attached patch this oneSplinter Review
Didn't even compile this yet.
Could someone still try this.
Attachment #403093 - Attachment is obsolete: true
Actually there is still one thing to try if the patch doesn't help.
Attached patch this waySplinter Review
(In reply to comment #16)
> Created an attachment (id=403094) [details]
> this one
> 
> Didn't even compile this yet.
> Could someone still try this.

With this patch it builds fine for me and brings back the tabs. But it seems something today breaked the search completely, because since today my workaround (Comment #11) and your patch doesn't bring back faceted search. :( And since today the search window is completely unusable.
But I will try also your "this way" patch...
(In reply to comment #19)
> But I will try also your "this way" patch...
Same result as "this one" patch.
Hm, there is something spooky. With your patch faceted search doesn't work for me. Than I opened one time a 3.0pre build. After that faceted search workes in the 3.1a1pre build with your patch ("this way"). So I can now say, your patch fixed tabs and search for me.

I don't know if this is related to this, but I now see a lot of errors in the error console "this._treeElement is undefined" for global/content/globalOverlay.js and another very long error message if I open the addressbook.
Comment on attachment 403095 [details] [diff] [review]
this way

Ok, based on FakeTreeBoxObject in folderDisplay.js we should take this, I think.

(IMHO FakeTreeBoxObject looks pretty ugly hack, though I don't know that code)
Attachment #403095 - Flags: review?(neil)
(In reply to comment #22)
> (IMHO FakeTreeBoxObject looks pretty ugly hack, though I don't know that code)
Very ugly... could they not at least fix their code to implement nsIBoxObject::GetElement to avoid having to return the wrong box object?
Comment on attachment 403095 [details] [diff] [review]
this way

Assuming I understood smaug correctly on IRC, the correct thing to do is to fix the FakeTreeBoxObject to fake a BoxObject as well as a TreeBoxObject.
Attachment #403095 - Flags: review?(neil) → review-
That would be better, yes, IMHO.
Does that sound reasonable to mailnews developers?
Though, actually, I think we assume elsewhere that nsIBoxObject implements nsPIBoxObject
Attachment #403095 - Flags: review?(neil)
Comment on attachment 403095 [details] [diff] [review]
this way

Neil, could you reconsider?
If the Thunderbird code created a dummy <tree> element instead of a <vbox>, then its boxObject would be a real nsITreeBoxObject, wouldn't it?
(In reply to comment #22)
> (IMHO FakeTreeBoxObject looks pretty ugly hack, though I don't know that code)

FakeTreeBoxObject is indeed a hideous hack.  I don't think toolkit or mozilla-central should address the needs of the hack.  It evolved just enough to do what is required in 1.9.1 and it was no surprise that it broke.

The 'right' solution continues to be comment #9.  The right hack fix here would be to fix FakeTreeBoxObject, but it's most important that we don't break our 1.9.1 stuff at all.

If someone wants to provide a fix along those lines, it'd be nice, but until tb drivers prioritize this, I'm going to let the tests keep burning or whatever they do for 3.1.
Note, I expect the Bug 516237 will land 1.9.1 in some form, so would that break
TB3? Or should we take attachment 403095 [details] [diff] [review] to trunk and to branches too?
It will indeed break us; consider this "drivers prioritizing" as much as is possible, whether it's for more faking of the fakeness of FakeTreeBoxObject or for nsTreeSelection tolerating our oddness.
Flags: blocking-thunderbird3+
Priority: -- → P1
Summary: [TB3.1] faceted search/message tabs broken → faceted search/message tabs broken by bug 516237
Target Milestone: --- → Thunderbird 3.0rc1
Whiteboard: [no l10n impact]
I'll take a look at this.
Assignee: nobody → bienvenu
nothing landed in 1.9.1.4, afaik, and that's what we're shipping with, so this isn't a 3.0 blocker. I might try to have a fix in our pocket in case we do ship with a newer gecko for some reason, and that gecko has the change that breaks us. We'll need it for 3.1 at least.
Flags: blocking-thunderbird3+ → blocking-thunderbird3.1+
Summary: faceted search/message tabs broken by bug 516237 → faceted search/message/any tabs broken by bug 516237
Duplicate of this bug: 523321
this fixes the issue - I don't know if the nsIBoxObject methods really need to do anything, or if they can simply be no-ops. Right now they just call the domNode methods. If they should be no-ops, let me know, and I can change them. It would be nice to fix the trunk sooner rather than later...
Could the patch for TB land before I'll land bug 516237 to 1.9.1.
Or, actually, for 1.9.1 we may want to take bug 516237 and 
https://bugzilla.mozilla.org/attachment.cgi?id=403095
Duplicate of this bug: 516905
Duplicate of this bug: 517240
Duplicate of this bug: 528461
Duplicate of this bug: 529448
Attachment #410093 - Flags: review?(bugmail)
Comment on attachment 410093 [details] [diff] [review]
possible fix
[Checkin: Comment 44]

I've just run this patch on my Mac. It seems to fix the general issue of tabs not working on trunk, running with mozmill also shows test passes at 224 and fails 18.

The current trunk mozmill passes are 78 and failures 159.

So getting this in would be a big help in cleaning up the test failures there and make trunk a bit more workable for those of us who need to deal with tabs.
Comment on attachment 410093 [details] [diff] [review]
possible fix
[Checkin: Comment 44]

Looks fine by me.  I must admit to being confused as to whether we need/will need this for 1.9.1 or not, but I think we might as well land it on comm-1.9.1.
Attachment #410093 - Flags: review?(bugmail)
Attachment #410093 - Flags: review+
Attachment #410093 - Flags: approval-thunderbird3.0.1?
fix checked in to the trunk.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: Thunderbird 3.0rc1 → Thunderbird 3.1a1
I don't think we need it for 1.9.1, because Olli lost faith in us landing anything, and landed the doesn't-break-us alternative there.
Attachment #410093 - Attachment description: possible fix → possible fix [Checkin: Comment 44]
Comment on attachment 403095 [details] [diff] [review]
this way

Olli, I'm assuming with the changes on this bug and bug 516237 this review request is no longer needed (especially as we've closed this bug now) - hence cancelling request. If it is needed, please feel free to re-request or move to a different bug.
Attachment #403095 - Flags: review?(neil)
Comment on attachment 410093 [details] [diff] [review]
possible fix
[Checkin: Comment 44]

Whilst it shouldn't hurt anything, I don't think we need to risk taking this on branch as afaict the fix for the other bug on 1.9.1 isn't going to affect us (we can always come back and revisit this decision if necessary).
Attachment #410093 - Flags: approval-thunderbird3.0.1? → approval-thunderbird3.0.1-
Depends on: 534424
You need to log in before you can comment on or make changes to this bug.