Closed
Bug 638801
Opened 13 years ago
Closed 13 years ago
Add an extra onLoad argument to contentTab and chromeTab arguments
Categories
(Thunderbird :: Toolbars and Tabs, defect)
Thunderbird
Toolbars and Tabs
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 3.3a3
People
(Reporter: protz, Assigned: protz)
Details
Attachments
(1 file, 2 obsolete files)
1.11 KB,
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
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.
Attachment #516898 -
Flags: review?(bwinton)
Assignee | ||
Updated•13 years ago
|
Attachment #516898 -
Flags: review?(bwinton) → review?(bugmail)
Assignee | ||
Comment 1•13 years ago
|
||
Sorry, forgot to qrefresh before uploading the patch.
Attachment #516898 -
Attachment is obsolete: true
Attachment #516898 -
Flags: review?(bugmail)
Attachment #516902 -
Flags: review?(bugmail)
Comment 2•13 years ago
|
||
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...
Attachment #516902 -
Flags: review?(bugmail)
Assignee | ||
Comment 3•13 years ago
|
||
Whoops I'm sorry, something went horribly wrong when uploading the patch. This of course isn't what I'm trying to get approved =).
Assignee | ||
Comment 4•13 years ago
|
||
Mark, this should be your area of expertise.
Attachment #516902 -
Attachment is obsolete: true
Attachment #517293 -
Flags: review?(bugzilla)
Comment 5•13 years ago
|
||
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!).
Attachment #517293 -
Flags: review?(bugzilla) → review+
Assignee | ||
Comment 6•13 years ago
|
||
No prob. Checked-in with extra comments. http://hg.mozilla.org/comm-central/rev/99b7adef97b7
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.3a3
You need to log in
before you can comment on or make changes to this bug.
Description
•