A number of the dependents of bug 1080942 need a new event framework in UITour, to allow parts of Firefox to tell a tour webpage that something happened. Let's use this bug to figure out what that should look like, so it's not spread among those other existing bugs. There are basically 3 parts to this: * Define a content API for tour pages to register and receive events * Define a chrome API for Firefox code to easily trigger such events * Implement the UITour.jsm / uitour.js code to glue #1 and #2 together I think it would make sense to base this on something similar to DOM event listeners, especially on the content-exposed side. [But minimally so, no bubbling/capturing/cancelable etc. The intent isn't to exactly clone addEventListener] So, something like: * addTourListener(eventType, callback) e.g. UITour.addTourListener("party", function onParty(event) { ... }); where |event| is a JS object with event.type == "party" and event.data an arbitrary object supplied from chrome. * removeTourListener(eventType, callback) (If we even need it; I'd suspect we don't) * dispatchTourEvent(eventType, data) [from chrome code] e.g. dispatchTourEvent("party", { likeItIs: 1999 }) The UITour module would be responsible for routing dispatchTourEvent() calls to any open tour page. Consider this as starting point for discussion, I'm not beholden to any specifics.

(In reply to Justin Dolske [:Dolske] from comment #0) > I think it would make sense to base this on something similar to DOM event > listeners, especially on the content-exposed side. [But minimally so, no > bubbling/capturing/cancelable etc. The intent isn't to exactly clone > addEventListener] So, something like: > > * addTourListener(eventType, callback) > > e.g. UITour.addTourListener("party", function onParty(event) { ... }); > where |event| is a JS object with event.type == "party" and event.data an > arbitrary object supplied from chrome. > > * removeTourListener(eventType, callback) > > (If we even need it; I'd suspect we don't) Something along these lines sounds good to me, +1

Simplified this quite a bit, to avoid having to track individual listeners. In chrome, you can use one of the following to send a notification to Tour pages: UITour.notify("some-event-happened"); UITour.notify("some-event-happened", "and this is more info"); UITour.notify("some-event-happened", {data: "oh my", moreData: "beards"}); This will send a notification to *all* pages that are using the UITour API. With the usual disclaimer about automatic teardown, and having to use the page visibility API. Such pages will always get a DOM event dispatched to them if UITour.jsm knows about them, and will get events for all notifications - it's up to the JS library to listen to them or not. In content, use the JS library like so: Mozilla.UITour.observe(function(event, params) { ... }); Or to remove that notification callback: Mozilla.UITour.observe(null); I've also added a "ping" action - I needed something like this for testing, and figured it would generally be useful. All it does is make UITour.jsm aware that the page is a Tour page, and fires an event back to the page. Used like: Mozilla.UITour.ping(function() { ... }); This JS library does *not* provide a way to filter notifications, or listen only for specific notifications - it's currently up to the listener to handle that.

Comment on attachment 8530807 [details] [diff] [review] Patch v1 Overall this looks fine to me - I'm happy to write a generic handleEvent type listener, which can then delegate out to different functions per event. This should do the job nicely.

Comment on attachment 8530807 [details] [diff] [review] Patch v1 Review of attachment 8530807 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/base/content/content-UITour.js @@ +84,4 @@ > bubbles: true, > detail: Cu.cloneInto(detail, doc.defaultView) > }); > + debugger; Leftover :P ::: browser/modules/UITour.jsm @@ +580,5 @@ > }).then(null, Cu.reportError); > break; > } > + > + case "ping": { Can you explain more about what ping is for? I think we currently use getConfiguration for something similar.

Comment on attachment 8530807 [details] [diff] [review] Patch v1 Review of attachment 8530807 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/modules/test/uitour.js @@ +75,5 @@ > + var data = {}; > + if (callback) { > + data.callbackID = _waitForCallback(callback); > + } > + _sendEvent('ping', data); Nit: double quotes

(In reply to Matthew N. [:MattN] from comment #5) > Nit: double quotes Heh, actually, it's the double quotes in that file that are wrong. That file follows the webdev style guide: http://mozweb.readthedocs.org/en/latest/reference/js-style.html#quotes

(In reply to Matthew N. [:MattN] from comment #4) > Can you explain more about what ping is for? I think we currently use > getConfiguration for something similar. Discussed this over Vidyo: Basically, yes, that. But specifically for that case, and without doing any work. Though, I forgot to mention that I think it will become more important with bug 941428.

(In reply to Blair McBride [:Unfocused] from comment #6) > (In reply to Matthew N. [:MattN] from comment #5) > > Nit: double quotes > > Heh, actually, it's the double quotes in that file that are wrong. That file > follows the webdev style guide: > http://mozweb.readthedocs.org/en/latest/reference/js-style.html#quotes That style guide also commands 4-space indents, for instance, which is different from both our common style and what's being done in this very file. Now, I'm actually asking you to use 4-space indents. I'd rather question the usefulness of trying to follow a style guide that was written down from external people who are working on a different code base.

Comment on attachment 8530807 [details] [diff] [review] Patch v1 Review of attachment 8530807 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/modules/UITour.jsm @@ +1554,5 @@ > + while (winEnum.hasMoreElements()) { > + let window = winEnum.getNext(); > + if (window.closed) > + continue; > +debugger; Leftover `debugger` here too

One thing I'm not 100% clear on is how the Chrome will know *when* to send an event. For example, a web page might need to know the status of the FTE tour when the page loads, but also when tab visibility changes etc (we need to hide info panels and other things when the tab is not visible). I can't test this just yet as the first notifications have not yet landed in Nightly, but I wondered if things like this can be handled already (or if they are even in scope).

The notifications to the page only get sent when they happen. Querying status for anything is outside the scope of this bug - this is just for the framework to send arbitrary notifications. For the ability to query the status of anything, we'd need a new bug to hook it up - it'd be using the getConfiguration API we already have. As far as the current context of Loop-related things goes, we could add a way to query whether a room is currently open. But I don't think it makes sense for any of the other current Loop UITour event bugs, as they're all stateless events.

Comment on attachment 8530807 [details] [diff] [review] Patch v1 [Triage Comment] Needed for Fx35 Hello tour, all code is tour-specific.