638801 - Add an extra onLoad argument to contentTab and chromeTab arguments

Status: RESOLVED FIXED

(Thunderbird :: Toolbars and Tabs, defect)

Description

[Jonathan Protzenko [:protz]]

Attachment: A patch that enables the required functionality

Problem:
--------

  Outer code opens a tab, and wishes the chrome page that's loaded in the tab to do something with its outer data once the chrome page is loaded. The data held by the outer code might be a closure, some component that the outer code has instantiated, some parameter, or whatever...

Solution 1:
-----------

  Outer code (myCodeModule.jsm):
    1: tabmail.openTab("chromeTab", { chromeUrl: ... });
    2: tab.browser.contentWindow.myFunc(myData);
  Inner code (myChromePage.html):
    3: function myFunc(aMyData) {
    4:   ...
    5: }

  This is not a solution because, as http://hsivonen.iki.fi/about-blank/ tells
  us, the <browser> that makes the chrome tab will start loading about:blank,
  and we have no guarantee about the sequence of events. We'd like to make sure
  that line 2: executes after the chrome page has been fully loaded and is
  operational, but AFAIK, we have no such guarantee.

Solution 2:
-----------

  Outer code (myCodeModule.jsm):
    1: tabmail.openTab("chromeTab", { chromeUrl: ... });
    2: tab.browser.myData = ...;
  Inner code (myChromePage.html):
    3: document.addEventListener("onload", function _myCallback () {
    4:   let myData = top.myData;
    5: });

  This is not a solution because, as http://hsivonen.iki.fi/about-blank/ tells
  us, the <browser> that makes the chrome tab will start loading about:blank,
  and we have no guarantee about the sequence of events. We'd like to be sure
  that _myCallback will never be called between lines 1 and 2, but AFAIK, we
  have no such guarantee.
  
Other non-solutions:
--------------------

  - Pass the data as a URL parameter: not feasible, because the data cannot be
    easily stringified.
  - Use a global array, store the data in the array, pass the index to the inner
    chrome page, have the chrome page retrieve its data using the global array.
    Ugly.  
    
Proposed solution:
------------------

  Outer code:
    1: tabmail.openTab("chromeTab", {
    2:   chromeUrl: ...,
    3:   onLoad: function (aEvent, aBrowser) {
    4:     aBrowser.contentWindow.doStuff(withMyData);
    5:   },
    6: });
  Inner code:
    1: function doStuff(aMyData) { ... }

  This solution has the advantage of offering all the guarantees we need. Plus,
  it is intuitive, and in the course of developing a few addons, I've wanted
  this everytime I've been interacting with content tabs. This solution is
  implemented by the patch attached.

I might be missing something, but I remember discussing this extensively with
bwinton (CC'd) over IRC, and if I recall correctly, we both came to the
conclusion that I had to submit such a patch or duplicate the chromeTab
functionnality in my own code and add the required hook.

Comment 1

[Jonathan Protzenko [:protz]]

Attachment: The right patch

Sorry, forgot to qrefresh before uploading the patch.

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 516902 [details] [diff] [review]
The right patch

content tabs are the domain of Standard8, but he will probably want a patch that is more than a bunch of dump statements.  (Ask him for review on the actual patch.)  Your previous patch looked like it did more...

Comment 3

[Jonathan Protzenko [:protz]]

Whoops I'm sorry, something went horribly wrong when uploading the patch. This of course isn't what I'm trying to get approved =).

Comment 4

[Jonathan Protzenko [:protz]]

Attachment: Hopefully this is the right patch this time

Mark, this should be your area of expertise.

Comment 5

[Mark Banner (:standard8)]

Comment on attachment 517293 [details] [diff] [review]
Hopefully this is the right patch this time

Ok, looks like this is necessary. Could you add a bit of documentation before the openTab function to detail what the possible arguments are please? (yes, I should have added that before!).

Comment 6

[Jonathan Protzenko [:protz]]

No prob. Checked-in with extra comments. http://hg.mozilla.org/comm-central/rev/99b7adef97b7