1236229 - [experiment] The showInfo API in UITour.jsm should accept JavaScript callbacks rather than callback IDs

Status: RESOLVED FIXED

(Firefox :: Tours, defect, P1)

Description

[:Paolo Amadini]

If the showInfo API in UITour.jsm accepts JavaScript callbacks instead of callback IDs, just like it already happens for the button callbacks, then chrome code could easily use them, and there would be no need to pass the message manager to the function anymore, resulting in cleaner code flow.

The close button callback will be used in bug 1231757.

Comment 1

[:Paolo Amadini]

Attachment: MozReview Request: Bug 1236229 - The showInfo API in UITour.jsm should accept JavaScript callbacks rather than callback IDs.

Review commit: https://reviewboard.mozilla.org/r/29341/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/29341/

Comment 2

[Alex Gibson [:agibson]]

Is this a breaking change to the current API? 

Please note that showInfo is already in use extensively on mozorg, so breaking changes should be avoided unless absolutely necessary. We also need to maintain compatibility for older versions of Firefox.

Comment 3

[Alex Gibson [:agibson]]

Just to be clear, I'm only asking in terms of the web content API that mozorg uses here, as opposed to anything internal in Firefox itself.

Comment 4

[Matthew N. [:MattN]]

Comment on attachment 8703344 [details]
MozReview Request: Bug 1236229 - The showInfo API in UITour.jsm should accept JavaScript callbacks rather than callback IDs.

https://reviewboard.mozilla.org/r/29341/#review26121

::: browser/components/uitour/UITour.jsm:1412
(Diff revision 1)
>     * Show an info panel.
>     *
>     * @param {ChromeWindow} aChromeWindow
> -   * @param {nsIMessageSender} aMessageManager
> +   * @param {nsIMessageSender} aMessageManager (This is currently unused.)
>     * @param {Node}     aAnchor

Feel free to remove the argument altogether and update references to this method. I think there's only a few.

IIUC this isn't going to change the usage from web content, only consumers of the JSM.

Comment 5

[:Paolo Amadini]

Tryserver build with the argument removed:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=eee393cfb3f3

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/37795691ec97

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/37795691ec97

Comment 8

[Mayur Patil]

[bugday-20160323]

Status: RESOLVED,FIXED -> UNVERIFIED

Comments:
STR: Not clear.
Developer specific testing

Component: 
Name			Firefox
Version			46.0b9
Build ID		20160322075646
Update Channel          beta
User Agent		Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS			Windows 7 SP1 x86_64

Expected Results: 
Developer specific testing

Actual Results: 
As expected