As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
Last Comment Bug 690414 - syncNotification.xml leaks 'Observers' and 'Notifications' into the global scope
: syncNotification.xml leaks 'Observers' and 'Notifications' into the global scope
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 10
Assigned To: Dão Gottwald [:dao]
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-29 10:18 PDT by Dão Gottwald [:dao]
Modified: 2011-10-03 19:22 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (3.14 KB, patch)
2011-09-29 10:18 PDT, Dão Gottwald [:dao]
gavin.sharp: review+
Details | Diff | Splinter Review

Description User image Dão Gottwald [:dao] 2011-09-29 10:18:16 PDT
Created attachment 563449 [details] [diff] [review]
patch
Comment 1 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2011-09-29 10:26:17 PDT
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?
Comment 2 User image Dão Gottwald [:dao] 2011-09-29 10:56:25 PDT
Possibly... Of course, there should be no global Weave object in the first place. (It should at least be called Sync.)
Comment 3 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2011-09-29 11:12:53 PDT
I don't think adding 1 more reference to it will significantly hinder efforts to rename it.
Comment 4 User image Dão Gottwald [:dao] 2011-09-29 11:39:15 PDT
No, but I don't know that it needs to be in browser.js at all. I haven't looked into this lately.
Comment 5 User image Marco Bonardo [::mak] 2011-10-03 07:57:26 PDT
https://hg.mozilla.org/mozilla-central/rev/e4ee17a3f362
Comment 6 User image Philipp von Weitershausen [:philikon] 2011-10-03 19:22:20 PDT
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).

Note You need to log in before you can comment on or make changes to this bug.