608981 - Enable building of places in Thunderbird, but disable history by default

Status: RESOLVED FIXED

(Thunderbird :: Build Config, defect)

Description

[Mark Banner (:standard8)]

To move the Thunderbird build closer to Firefox, and to easily enable interesting things like favicons for content tabs, we should start building places. Extensions should also be able to use it if they wish to.

I can't see a reason at the moment for us to enable storing of history by default, so I think we should disable it by default but keep reasonable defaults in place. I believe that favicon storing/usage will still work even if the history is disabled (I think it relies on other prefs).

Comment 1

[David Ascher (:davida)]

Clarification question: can we do browser navigation (back/forward) w/o history?  Those seem like likely useful things in site-specific contentTabs.

Comment 2

[Mark Banner (:standard8)]

(In reply to comment #1)
> Clarification question: can we do browser navigation (back/forward) w/o
> history?  Those seem like likely useful things in site-specific contentTabs.

With places built, it should be the same as setting Firefox to "Never Remember History".

Without places, I'm not sure. Although you said earlier that it works on trunk but not 1.9.2...

Comment 3

[Mark Banner (:standard8)]

Attachment: Enable places in Thunderbird

This patch enables building of places within Thunderbird, but history disabled by default.

It needs a core patch to pass tests, as the core tests assume history is enabled by default. I'll attach a patch in a moment that fixes all but one of those tests.

The remaining test that I haven't fixed currently hangs and I haven't worked out why yet.

Comment 4

[Mark Banner (:standard8)]

Attachment: Force enabling of places in core xpcshell-tests

test_doSetAndLoadFaviconForPage_failures.js is the test with an issue here. It is currently disabled with an early return.

Comment 5

[David Ascher (:davida)]

yes, ignore comment #1.

Comment 6

[Mark Banner (:standard8)]

Comment on attachment 487733 [details] [diff] [review]
Force enabling of places in core xpcshell-tests

The full patch is now on bug 610974.

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 487731 [details] [diff] [review]
Enable places in Thunderbird

With the patch in bug 610974, all the tests pass on try server. I believe this is everything we need.

Comment 8

[Phil Ringnalda (:philor)]

Comment on attachment 487731 [details] [diff] [review]
Enable places in Thunderbird

Looks good to me.

Comment 9

[Nomis101]

If places is enabled in TB, will this fix the following error (I see everytime I start TB)?

Error: Places database may be locked: [Exception... "Component returned failure code: 0x80040154 (NS_ERROR_FACTORY_NOT_REGISTERED) [nsIDocShellHistory.useGlobalHistory]"  nsresult: "0x80040154 (NS_ERROR_FACTORY_NOT_REGISTERED)"  location: "JS frame :: chrome://messenger/content/specialTabs.js :: anonymous :: line 170"  data: no]
File: chrome://messenger/content/specialTabs.js
Line: 172


Or is this unrelated?

Comment 10

[Mark Banner (:standard8)]

(In reply to comment #9)
> If places is enabled in TB, will this fix the following error (I see everytime
> I start TB)?

Most likely, though I haven't tested it yet.

Comment 11

[Mark Banner (:standard8)]

Just to keep folks updated, this landed but got backed out again because I hadn't taken account of the c++ tests - bug 614550 fixes the failure, so I'll attempt to re-land following that.

Comment 12

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/a42338acc7db