596799 - document.getElementById("sync-button") is null

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Philipp von Weitershausen [:philikon]]

Attachment: v1

doslke reported this after bug 594488:

************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "'[JavaScript Error: "document.getElementById("sync-button") is nu
ll" {file: "chrome://browser/content/browser.js" line: 5035}]' when calling meth
od: [nsIObserver::observe]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERRO
R_WITH_DETAILS)"  location: "JS frame :: resource://services-sync/ext/Observers.
js :: anonymous :: line 122"  data: yes]
************************************************************

Can't actually reproduce it myself, but there's one place where we don't check for gBrowser.

Comment 1

[Philipp von Weitershausen [:philikon]]

(In reply to comment #0)
> Can't actually reproduce it myself,

Strike that. I see it now and the patch fixes it.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 475701 [details] [diff] [review]
v1

This is necessary but not sufficient - can we morph this bug to cover bug 594488 comment 16 and subsequent comments?

Comment 3

[Philipp von Weitershausen [:philikon]]

(In reply to comment #2)
> This is necessary but not sufficient - can we morph this bug to cover bug
> 594488 comment 16 and subsequent comments?

Can do. Working on new patch.

Comment 4

[Philipp von Weitershausen [:philikon]]

Attachment: v2

Make sure that the Sync button is only accessed when it actually exists in the toolbar. Also fix its entry in browserShared.inc.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 475714 [details] [diff] [review]
v2

>diff --git a/browser/base/content/browser-syncui.js b/browser/base/content/browser-syncui.js

>+    if (!gBrowser)

>+    if (!button)

Technically these checks are redundant, but I guess it doesn't really hurt to be explicit about the two cases (and avoid the getElementById call in the hiddenWindow case).

>   // Functions called by observers
>   onActivityStart: function SUI_onActivityStart() {

>     //XXXzpao Followup: Do this with a class. (bug 583384)

This comment is no longer relevant... remove while you're here?

Comment 6

[Philipp von Weitershausen [:philikon]]

Attachment: v2.1

Address review comment: Remove no longer needed comment.

Comment 7

[Philipp von Weitershausen [:philikon]]

http://hg.mozilla.org/mozilla-central/rev/dd872c73e5c4