Last Comment Bug 636110 - Update signature of duplicateTabIn() stub to match match Firefox 4.0.
: Update signature of duplicateTabIn() stub to match match Firefox 4.0.
Status: RESOLVED FIXED
: regression
Product: SeaMonkey
Classification: Client Software
Component: Tabbed Browser (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.1b3
Assigned To: Philip Chee
:
Mentors:
Depends on: 595483
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-23 02:11 PST by Philip Chee
Modified: 2011-02-24 20:27 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v1.0 inc SessionRestore fix. (2.75 KB, patch)
2011-02-23 19:19 PST, Philip Chee
neil: review+
misak.bugzilla: feedback+
Details | Diff | Splinter Review

Description Philip Chee 2011-02-23 02:11:47 PST
duplicateTabIn() is the Firefox equivalent of our OpenSessionHistoryIn(). After I implemented a stub duplicateTabIn() for extension compatibility, Dao decided that our API was saner. This bug just updates our stub.
Comment 1 Philip Chee 2011-02-23 19:19:05 PST
Created attachment 514710 [details] [diff] [review]
Patch v1.0 inc SessionRestore fix.

>    duplicateTab: function sss_duplicateTab(aWindow, aTab, aDelta, aRelated) {
>      if (!aTab.ownerDocument || !aTab.ownerDocument.defaultView.__SSi ||
>          aWindow && !aWindow.getBrowser)
>        throw (Components.returnCode = Components.results.NS_ERROR_INVALID_ARG);
>  
>      var tabState = this._collectTabData(aTab, true);
>      var sourceWindow = aTab.ownerDocument.defaultView;
>      this._updateTextAndScrollDataForTab(sourceWindow, aTab.linkedBrowser, tabState, true);
> -    this._sendWindowStateEvent(aWindow, "Busy");
>      tabState.index += aDelta;
> +    tabState.index = Math.max(1, Math.min(tabState.index, tabState.entries.length));
>  
>      if (aWindow) {
> +      this._sendWindowStateEvent(aWindow, "Busy");

_sendWindowStateEvent falls over when aWindow is null. Introduced in:

http://hg.mozilla.org/comm-central/rev/c18dddea67de
Bug 633722 - Port Bug 615394 [Session Restore should notify when it is beginning and ending a restore]
Oops.
Comment 2 Misak Khachatryan 2011-02-23 22:38:09 PST
Comment on attachment 514710 [details] [diff] [review]
Patch v1.0 inc SessionRestore fix.

Perhaps we should modify it to use current window if aWindow is null ...
Comment 3 neil@parkwaycc.co.uk 2011-02-24 01:29:14 PST
(In reply to comment #2)
> Perhaps we should modify it to use current window if aWindow is null ...
No, because in that case we're creating a new window, and therefore restoreWindow will call _sendWindowStateEvent on the new window for us, right?

Incidentally I notice that undo close tab calls _sendWindowStateEvent even when using the tabbrowser to restore the tab.
Comment 4 Philip Chee 2011-02-24 02:09:41 PST
> > Perhaps we should modify it to use current window if aWindow is null ...
> No, because in that case we're creating a new window, and therefore
> restoreWindow will call _sendWindowStateEvent on the new window for us, right?
Also my reading of the code indicates that there should be a matching _sendWindowStateEvent(aWindow, "Ready") on the same window so it's no use sending only half of the notifications.
Comment 5 Philip Chee 2011-02-24 02:17:20 PST
Pushed to comm-central.
http://hg.mozilla.org/comm-central/rev/aa25d31c8221
Comment 6 neil@parkwaycc.co.uk 2011-02-24 16:23:27 PST
Comment on attachment 514710 [details] [diff] [review]
Patch v1.0 inc SessionRestore fix.

>-  OpenSessionHistoryIn(aWhere, delta, aTab)
>+  OpenSessionHistoryIn(aWhere, aDelta, aTab)
[Bah, I failed to notice the missing semicolon]
Comment 7 Philip Chee 2011-02-24 20:27:58 PST
> [Bah, I failed to notice the missing semicolon]

Added semicolon:
http://hg.mozilla.org/comm-central/rev/8310d0fd89a6

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