435174 - Today Pane not displayed after startup when using Spamato extension [Error: LOG is not a function]

Status: RESOLVED FIXED

(Calendar :: Lightning Only, defect)

Description

[Stefan Sitter [:ssitter]]

From <http://forums.mozillazine.org/viewtopic.php?t=657592>

When Lightning 0.9pre and Spamato 0.99.9 <http://sourceforge.net/projects/spamato/> <http://downloads.sourceforge.net/spamato/spamato4thunderbird-0.99.9.xpi> are installed the Today Pane is no longer displayed after startup.

Regression range: Works in Lightning 0.9pre (20080505) + Spamato 0.99.9
                  Fails in Lightning 0.9pre (20080506) + Spamato 0.99.9

Error Console shows:

  Error: LOG is not a function
  Source File: chrome://calendar/content/calendar-views.js
  Line: 429

The LOG() calls were added by Bug 401693. 
Unfortunately Spamato already declares a global variable named LOG: <http://spamato.cvs.sourceforge.net/spamato/spamato4thunderbird/addon/spamato/chrome/content/spamato/communication.js?view=markup#l_43>

Removing the LOG() calls seems to fix the issue at the moment. A proper fix might be to move the global methods to calendar namespace e.g. by renaming it to calLOG().

Comment 1

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Similar problems made us use Component.classes instead of Cc. I guess we could rename our functions every time some other extension uses the same name, but I'd prefer namespace support for js, which I believe only exists in the future, in js2.

I kind of liked the fact that LOG() was so short and prominent. Maybe we could rather persuade Spamato to use a different variable?

Comment 2

[Stefan Sitter [:ssitter]]

Honestly, if I would be a Spamato developer I'd just say: "Why should we change? Our latest release works with Lightning 0.7 and 0.8. Now Lightning does a change and breaks." Nevertheless it might be worth to ask them.

For the moment I'd suggest to replace the LOG() calls with plain dump() calls or to remove them. Thoughts?

Comment 3

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

We use LOG() for a reason (i.e show the message in the error console instead of only the system console).

I wonder why it worked before? Did we not have LOG() calls in chrome script?

Comment 4

[Daniel Boelzle [:dbo]]

IMO the only feasible opportunity is really to prefix all (shared) UI code stuff (or alternatively scope all that stuff in a top-level object, e.g. named "cal"). While namespace support would be favourable (however that looks like), we even then need to adopt the UI code to define lookup preference.

Comment 5

[Martin Schröder [:mschroeder]]

Should we fix this for better compatibility with other extensions?

Comment 6

[Daniel Boelzle [:dbo]]

I agree. we should go with namespace support or scope code into a global "cal" object. However, I don't think this should block 0.9, but patches (at least partial ones for e.g. calendar-views.js) are welcome.

Comment 7

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

I've heard we can fake namespace support using something like

var extensions = extensions || {};
var extensions.calendar = extensions.calendar || {};

with (extensions.calendar) {
    // Old code goes here
}

This change will ruin cvsblame, since we'll need to indent everything.

Comment 8

[Stefan Sitter [:ssitter]]

Or just replace the LOG() calls with simple dump() calls in the UI code.

Comment 9

[Stefan Sitter [:ssitter]]

Attachment: [checked in] remove the LOG() calls

This patch works around the issue by removing the LOG() calls in scheduleMidnightUpdate(). This way it's possible to work with Lightning. Timezone service, alarm service, itip email service work and successfully use LOG() to log to the Error Console.

The only component that fails seems to be the migration code. If the Spamato extension is installed and Lightning is installed for the very first time the migration wizard is not shown but Lightning starts correctly.

Comment 10

[Daniel Boelzle [:dbo]]

Comment on attachment 332941 [details] [diff] [review]
[checked in] remove the LOG() calls

r=dbo and checked in on HEAD and MOZILLA_1_8_BRANCH.

Comment 11

[Keno Albrecht]

Just found this thread by accident.

The next release of Spamato doesn't have any 'global' variables outside the "SPAMATO."-namespace. We had several difficulties also with other extensions and decided to use our own namespace to circumvent this and future problems.

It would anyway be a good idea to prefix your own code I suppose :-).

Comment 12

[Stefan Sitter [:ssitter]]

Resolving as FIXED. Current Lightning 0.9pre nightly builds should work now if the Spamato extension is installed. And according to comment #11 Lightning 0.8 should work again after the Spamato extension is updated.

Daniel, Philipp please file a follow-up bug if there should be more work done to avoid similar issues, e.g. by renaming or using namespace.