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

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
9 years ago
10 months ago

People

(Reporter: philikon, Assigned: philikon)

Tracking

unspecified
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 beta7+)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment, 2 obsolete attachments)

Posted patch v1 (obsolete) — Splinter Review
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.
Attachment #475701 - Flags: review?(dolske)
(In reply to comment #0)
> Can't actually reproduce it myself,

Strike that. I see it now and the patch fixes it.
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?
Attachment #475701 - Flags: review?(dolske) → feedback+
blocking2.0: --- → beta7+
(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.
Posted patch v2 (obsolete) — Splinter Review
Make sure that the Sync button is only accessed when it actually exists in the toolbar. Also fix its entry in browserShared.inc.
Assignee: nobody → philipp
Attachment #475701 - Attachment is obsolete: true
Attachment #475714 - Flags: review?(gavin.sharp)
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?
Attachment #475714 - Flags: review?(gavin.sharp) → review+
Posted patch v2.1Splinter Review
Address review comment: Remove no longer needed comment.
Attachment #475714 - Attachment is obsolete: true
http://hg.mozilla.org/mozilla-central/rev/dd872c73e5c4
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [qa-]
Component: Firefox Sync: UI → Sync
Product: Cloud Services → Firefox
You need to log in before you can comment on or make changes to this bug.