The default bug view has changed. See this FAQ.

syncNotification.xml leaks 'Observers' and 'Notifications' into the global scope

RESOLVED FIXED in Firefox 10

Status

()

Firefox
General
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: dao, Assigned: dao)

Tracking

Trunk
Firefox 10
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
Created attachment 563449 [details] [diff] [review]
patch
Attachment #563449 - Flags: review?(gavin.sharp)
Comment on attachment 563449 [details] [diff] [review]
patch

>diff --git a/browser/base/content/syncNotification.xml b/browser/base/content/syncNotification.xml

>+        Cu.import("resource://services-sync/notifications.js", temp);
>+        for each (var notification in temp.Notifications.notifications)

Looks like this could use Weave.Notifications?
Attachment #563449 - Flags: review?(gavin.sharp) → review+
(Assignee)

Comment 2

6 years ago
Possibly... Of course, there should be no global Weave object in the first place. (It should at least be called Sync.)
I don't think adding 1 more reference to it will significantly hinder efforts to rename it.
(Assignee)

Comment 4

6 years ago
No, but I don't know that it needs to be in browser.js at all. I haven't looked into this lately.
https://hg.mozilla.org/mozilla-central/rev/e4ee17a3f362
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 10
Thanks for fixing this, Dão!

(In reply to Dão Gottwald [:dao] from comment #4)
> No, but I don't know that it needs to be in browser.js at all. I haven't
> looked into this lately.

browser-syncui.js uses it a lot. It's the main entry point for Sync's UI-facing APIs. Note that instead of importing the JSMs in the first place, the exposed objects could've been accessed via the 'Weave' object (Weave.Svc.Obs and Weave.Notifications).
You need to log in before you can comment on or make changes to this bug.