Open Bug 482580 Opened 11 years ago Updated 3 years ago
Session store should provide a method for clearing out current tabs via set
+++ This bug was initially created as a clone of Bug #476928 +++ With bug 476928 fixed we have mostly a seamless change into private browsing mode. But having a tab at the end of the tabbar open still shows the first tab before the private browsing mode is entered. That ends up in kinda short flickering. Given by bug 476928 comment 19 we should try to investigate if we can make it even more smoother.
The tab you're seeing is probably the transistionState tab, which is necessary due to bug 476463, however I think there may be a way to flush the events on any given document, which would make this much smoother, cc'ing Boriss to see if there is such a thing.
Summary: Slow closing of tabs with the given testcase when entering Private Browsing mode → Further improvements for a seamless switch to Private Browsing mode
I think you mean bz. Boriss is a UX designer (Jennifer Boriss). :)
I'm not sure what comment 1 is asking, exactly.... Is the problem that the new tab is visible to the user and then we close it? Is what's desired here just locking down painting for all toplevel windows until after the transition is complete or some such?
Boris: Here is a description of what I see using the 1.9.1 B3 versus the trunk, which I think henrik is explaining. Using 1.9.1 B3, if I have a set of tabs open and launch PB mode, it chunks the tabs closed one at a time (I can see each one for a moment before they close, and the same thing happens when I exit PB mode and go back to regular mode) Using the trunk, if I have a set of tabs open and launch PB mode, it switches seamlessly to the mode without seeing all the open tabs close one at a time.
Oh. I have no idea why that difference would be there, offhand.
I think what Nochum meant was asking if there is any way to force an unload event on the set of currently open documents without closing down the tabs they're residing in, so that we can avoid opening an about:blank page just for the sake of getting the onload event reliably fired on the document.
Sure. For example, you could navigate all those documents to about:blank...
Yes, that's what we do now. Is there any other way to do this, preferably something which doesn't cause a repaint?
(In reply to comment #4) > Using 1.9.1 B3, if I have a set of tabs open and launch PB mode, it chunks the > tabs closed one at a time (I can see each one for a moment before they close, > and the same thing happens when I exit PB mode and go back to regular mode) Something has been changed here in the last two days. I cannot see see the behavior I've subscribed in comment 0 anymore. Instead we have the old behavior before the patch on bug 476928 was checked-in. I've removed the verified1.9.1 keyword there and we should wait with this bug until bug 476928 is really fixed on 1.9.1.
If we have a regression, I'd love a regression range, of course!
Unload events are synchronous (modulo alerts from beforeunload and the like), so I'm not sure why there's this much complexity here to start with...
Session Restore is async though afaik. We were reusing the tabs (thats how session store works) and that's where the problem was.
It looks like that the patch on bug 476928 doesn't have fixed it at all while on trunk it is fine and gives us a seamless switch over.
I've got a simple patch to fix most of this issue in SessionStore over in bug 476928 (failed to note this bug and could move it over if desired).
No, it's fine Simon. We could still use this bug afterward if we need some more investigation. Thanks!
Is there anything left to do in this bug or has Simon's patch resolved this? bz: if I reuse a tab and load a new url am I guaranteed that by the time the net piece of code runs onunload will have fired on the first document? Tab a has google.com loaded, I switch it to about:blank then run some unrelated code afterwards, am I guaranteed that google.com receive its unload event by the time the code runs (even if I didn't close the tab)?
> Tab a has google.com loaded, I switch it to about:blank then run some unrelated > code afterwards, am I guaranteed that google.com receive its unload event by the > time the code runs Not at all. In fact, you're guaranteed the opposite. The unload will happen asynchronously, when the about:blank load starts handing back data.
Simons new patch hasn't been checked into 1.9.1 yet. We are still waiting for approval. We have to wait until I can verify it with a nightly build.
(In reply to comment #18) ...which is why the tab add/remove hack is necessary. I guess we can use this bug to implement it in sessionstore, although that involves a bit of morphing :)
Now, with bug 476928 fixed on 1.9.1 it feels much better and I don't have something to add to this bug. Nochum, if you have ideas for further improvements feel free to morph the bug a bit.
(In reply to comment #21) > Now, with bug 476928 fixed on 1.9.1 it feels much better and I don't have > something to add to this bug. Nochum, if you have ideas for further > improvements feel free to morph the bug a bit. Do you mean that the speed matches that of trunk builds, or we still need further improvements?
Yes, that's why I have verified bug 476928 on 1.9.1. But see comment 20 which sounds like more improvements.
(In reply to comment #23) > Yes, that's why I have verified bug 476928 on 1.9.1. But see comment 20 which > sounds like more improvements. Nochum, would you like to morph this bug according to comment 20?
Component: Tabbed Browser → Session Restore
QA Contact: tabbed.browser → session.restore
Summary: Further improvements for a seamless switch to Private Browsing mode → Session store should provide a method for clearing out current tabs via setBrowserState
Version: 3.1 Branch → Trunk
Simon: does session store want this fixed? It sounds like it's something specific to private browsing, as the session won't be restored synchronously even with this method. The only advantage is that the current tabs will be removed synchronously, then the new state will be loaded asynchronously. This would be a messy api and I can't really understand why it fits in session store... Thoughts?
(In reply to comment #26) I'd actually not expect e.g. cookies being set by setBrowserState being overwritten by an onunload handler. So yes, we do want the issue from your comment #11 to be fixed inside SessionStore.
This is needed for bug 498648, patch will be there.
Bug 498648 was fixed without this, I'll look into this for .next anyhow.
No longer depends on: 498648
Assignee: highmind63 → nobody
You need to log in before you can comment on or make changes to this bug.