344587 - Dispatch tab open/close events in the correct order

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Simon Bünzli]

The underlying cause of bug 343422 should still be fixed:

When the last tab is closed (and the window is not), a new tab is added before the old tab is removed. The tab events are however dispatched in the inverse order (TabClose and then TabOpen) which might break other code in the future.

Comment 1

[Simon Bünzli]

Attachment: fix

Comment 2

[(not reading, please use [e-mail] instead)]

I agree simon, we should fix this.

cc'ing dietrich, in case changing this order might affect session restore in a way that I'm not thinking of.

Comment 3

[Simon Bünzli]

Looking through Dietrich's modifications to SessionStore, I don't see how this could have any effect:

On TabOpen we just attach event listeners to the tab and schedule a disk save operation; and on TabClose we fetch the data for that tab for the Restore... list, detach the listeners and also schedule a disk save operation. The only place where problems could be possible is during the data fetching - and that obviously doesn't depend on whether the event listeners on the new tab are already installed.

Comment 4

[Asaf Romano (gone)]

Comment on attachment 229152 [details] [diff] [review]
fix

Simon, can you update this patch to tip?

Comment 5

[(not reading, please use [e-mail] instead)]

simon / asaf:  I'll work on updating, reviewing and driving this in today.  sorry for the delay.

Comment 6

[(not reading, please use [e-mail] instead)]

Attachment: simon's patch, updated to branch (with comment changed)

Comment 7

[(not reading, please use [e-mail] instead)]

Comment on attachment 230513 [details] [diff] [review]
simon's patch, updated to branch (with comment changed)

this patch seems correct (and low risk) to me, but being so close to b2, I'd like to make sure dietrich and asaf agree.

Comment 8

[Asaf Romano (gone)]

Comment on attachment 230513 [details] [diff] [review]
simon's patch, updated to branch (with comment changed)

looks goog and safe to me, r=mano.

Comment 9

[Mike Connor [:mconnor]]

Comment on attachment 230513 [details] [diff] [review]
simon's patch, updated to branch (with comment changed)

three sets of eyes are totally enough!

Comment 10

[Mike Connor [:mconnor]]

seth landed this on trunk yesterday.

Comment 11

[(not reading, please use [e-mail] instead)]

> seth landed this on trunk yesterday

whoops, I just landed it now.

yesterday, in a state of confusion, I landed it on  branch only, and then backed myself after realizing I had sr from mconnor, not a=.

but I forgot to land it on the trunk!

Comment 12

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 230513 [details] [diff] [review]
simon's patch, updated to branch (with comment changed)

a=drivers. Please land on the MOZILLA_1_8_BRANCH.

Comment 13

[(not reading, please use [e-mail] instead)]

fixed on branch.

thanks again to simon for this fix.

Comment 14

[Simon Bünzli]

Verified using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b2) Gecko/20060828 BonEcho/2.0b2 ID:2006082803.