See comments from Neil in bug 318168 and bug 322898. If we do keep the TabSelect for the tabbox bindings, we should also dispatch TabOpen and TabClose events on them (note tabbox has support for adding and removing tabs in its API).

I don't think this blocks, it'd be nice to settle on an answer though. (Note that the tabbox select event didn't bubble in 1.5, that's a new change.)

(In reply to comment #1) >I don't think this blocks, it'd be nice to settle on an answer though. (Note >that the tabbox select event didn't bubble in 1.5, that's a new change.) IIRC it did because XUL elements used to ignore the canBubble flag.

attaching to the right bug...

Neil pointed out that the event should be dispatched from the tab, which the previous patch did not do.

Comment on attachment 228485 [details] [diff] [review] better patch updateCurrentBrowser is also called from removeTab (i didn't check why), and is also a pseudo-api here, I don't think we should let callers accidentally dispatch this event.

(In reply to comment #5) >I don't think we should let callers accidentally dispatch this event. I think that the TabSelect event should be dispatched however the tab is changed, and not just through user input, for which the select event suffices.

(In reply to comment #5) > (From update of attachment 228485 [details] [diff] [review] [edit]) > updateCurrentBrowser is also called from removeTab (i didn't check why), and is > also a pseudo-api here, I don't think we should let callers accidentally > dispatch this event. > Mano, does Neil's comments affect your thoughts?

(In reply to comment #6) > (In reply to comment #5) > >I don't think we should let callers accidentally dispatch this event. > I think that the TabSelect event should be dispatched however the tab is > changed, and not just through user input, for which the select event suffices. > I'm not sure what "user input" and "tah is changed" mean here. In which case, "select" isn't dispatched? Why should TabSelect be dispatched in that case?

(In reply to comment #8) > (In reply to comment #6) > > (In reply to comment #5) > > >I don't think we should let callers accidentally dispatch this event. > > I think that the TabSelect event should be dispatched however the tab is > > changed, and not just through user input, for which the select event suffices. > > > > I'm not sure what "user input" and "tah is changed" mean here. In which case, > "select" isn't dispatched? Why should TabSelect be dispatched in that case? > You close the current tab. As a result, a different tab gets selected. People who care about tab switches probably care about that indirectly-caused current-tab change as well as ones caused by normal methods (e.g. clicking a different tab), so dispatching the TabSelect is a good thing.

(In reply to comment #9) > > > > I'm not sure what "user input" and "tah is changed" mean here. In which case, > > "select" isn't dispatched? Why should TabSelect be dispatched in that case? > > > > You close the current tab. As a result, a different tab gets selected. People > who care about tab switches probably care about that indirectly-caused > current-tab. Are we sure "select" isn't dispatched in that case? I know the comment says so, but...

Well without auditing the code I guess there are four possibilities: 1) select events exactly match tab changes. This means that moving the TabSelect notification into tabbrowser will have no effect. 2) some tab changes don't generate select events, which is why removeTab calls updateCurrentBrowser 3) some tab changes generate duplicate select events, which is why updateCurrentBrowser checks that the browser isn't already current first 4) both 2) and 3) In the latter cases, moving the TabSelect notification into tabbrowser will make it more reliable and useful. Either way you can't lose :-)

Comment on attachment 228485 [details] [diff] [review] better patch OK, this will do for now. I would really like to see this on the branch before extensions start relying on the new tabbox event. Please file a bug on cleaning up the removeTab mess.

Comment on attachment 228485 [details] [diff] [review] better patch a=mconnor on behalf of drivers for 1.8 branch

Checked in on trunk and branch. Filed bug 351106 on the removeTab stuff.