Add events for watching the addition and removal of tabs

RESOLVED FIXED in mozilla1.8.1

Status

()

defect
P2
normal
RESOLVED FIXED
14 years ago
13 years ago

People

(Reporter: mano, Assigned: moco)

Tracking

({fixed1.8.1})

unspecified
mozilla1.8.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox2 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 obsolete attachments)

The current tabbrowser and tabbox bindings do not provide any supported way of watching the addition/removal of tabs (except DOMNodeInserted/Removed events). We should probably fire some events in the addTab and removeTab methods.
I have a patch for this.
Assignee: bugs.mano → bryner
Posted patch patch (obsolete) — Splinter Review
This adds events for tab creation, removal, and reordering (newtab, closetab, and movetab, respectively).  The event target is the tab that's affected.  For the movetab event, event.detail gives the old index of the tab.
Attachment #211237 - Flags: first-review?(bugs)
Would it make sense to allow the closetab listener cancel the operation?
Comment on attachment 211237 [details] [diff] [review]
patch

Can I get you to rename these to tab-open, tab-close and add a similar event for tab-select? (for completeness, currently to get select events you need to attach a select handler directly to mTabContainer, to avoid getting select events from web content).
Attachment #211237 - Flags: first-review?(bugs) → first-review-
Posted patch patch (obsolete) — Splinter Review
Now with better event names.  You might notice the reversal of the false and true arguments for initializing the "TabSelect" event vs. the "select" event.  The arguments for the select event don't make a lot of sense to me (doesn't bubble, but can cancel? cancelling it does nothing).  But to not break compatibility, I just left it and made the new event work in a sane way.
Attachment #211237 - Attachment is obsolete: true
Attachment #211633 - Flags: first-review?(bugs)
Comment on attachment 211633 [details] [diff] [review]
patch

I'm pretty sure we want the event on the tabs, not the tabpanels... Aside from that I've bitrotted a bunch of this, so I'm going to roll it into my soon-to-be-attached patch for reopening closed tabs
Attachment #211633 - Attachment is obsolete: true
Attachment #211633 - Flags: first-review?(bugs)
Depends on: undoclosetab
ok, sounds good.
Assignee: bryner → mconnor
Status: NEW → ASSIGNED
Flags: blocking-firefox2+
Priority: -- → P2
Target Milestone: --- → mozilla1.8.1
*** Bug 329840 has been marked as a duplicate of this bug. ***
Blocks: 331443
Whiteboard: SWAG: 0.25d
Blocks: 334839
load-balancing tabbrowser stuff to sspitzer
Assignee: mconnor → sspitzer
Status: ASSIGNED → NEW
gah.  s/.org/.com/!
Assignee: sspitzer → sspitzer
Comment on attachment 211633 [details] [diff] [review]
patch

>Index: tabbox.xml
I would have thought that all these new events are tabbrowser-specific and should therefore not be fired here.
(In reply to comment #12)
> (From update of attachment 211633 [details] [diff] [review] [edit])
> >Index: tabbox.xml
> I would have thought that all these new events are tabbrowser-specific and
> should therefore not be fired here.
> 

Seems like the kind of stuff that superreview is intended for.
accepting.  increasing swag though, because I'm no mconnor!

this blocks the undoclosetab that dietrich is working on, so I plan to get to this right away.
Status: NEW → ASSIGNED
Whiteboard: SWAG: 0.25d → SWAG: 1d
finally getting to this (sorry for the delay, dietrich!)

mconnor wrote (over email):

"there should be events for TabClose/TabOpen/TabSelect/TabMove added, and get the change to the tabbrowser-tabs binding that listens for DOMNodeRemoved/DOMNodeAdded while you're there"

working on that now.
"there should be events for TabClose/TabOpen/TabSelect/TabMove added, and get the change to the tabbrowser-tabs binding that listens for DOMNodeRemoved/DOMNodeAdded while you're there"

s/DOMNodeAdded/DOMNodeInserted

some questions:

1)

I'm having a problem with the DOMNodeInserted change from mconnor's previous patch (it breaks open group as tabs).

2)  in one of mconnor's earlier patches, he changed "spacer" to "box", but that broke open in a new tab.

+              // changing this to box breaks double clicking
+              // on the spacer to open a new tab
                 aEvent.originalTarget.localName == "spacer") {

3)

finally, the 2nd and 3rd params to initEvent() are for canBubble and cancelable, and I've done true, false (following mconnor's lead).  this makes sense to me, but I wanted to double check with mconnor. 
(In reply to comment #17)
>finally, the 2nd and 3rd params to initEvent() are for canBubble and
>cancelable, and I've done true, false (following mconnor's lead).  this makes
>sense to me, but I wanted to double check with mconnor. 
Triple-checking with Smaug ;-)

(From update of attachment 224502 [details] [diff] [review])
>Index: tabbox.xml
>+            // Support both the old "select" event and the new, better-named
>+            // "TabSelect" event.
Surely the TabSelect event belongs in tabbrowser.xml with the other events?
Comment on attachment 224502 [details] [diff] [review]
initial patch based on earlier patches from mconnor

>+
>+          var evt = document.createEvent("UIEvents");
>+          evt.initUIEvent("TabMove", true, false, window, oldPosition);
>+          aTab.dispatchEvent(evt);

I guess this is ok, but it would be great if someone adds some documentation about these
events.


(In reply to comment #17)
>finally, the 2nd and 3rd params to initEvent() are for canBubble and
>cancelable, and I've done true, false (following mconnor's lead).  this makes
>sense to me, but I wanted to double check with mconnor. 

Makes sense to me, because the return value of dispatchEvent is not used.
TabSelect is useful to non-tabbrowser consumers of tabbox.  If we move dnd reordering to to tabbox, as has been requested, we'd move TabMove there too, IMO.
(In reply to comment #20)
>TabSelect is useful to non-tabbrowser consumers of tabbox.
One event isn't enough for them?
Actually two, as both tabs and panels like to fire select events.
No longer depends on: undoclosetab
*** Bug 278288 has been marked as a duplicate of this bug. ***
Comment on attachment 224502 [details] [diff] [review]
initial patch based on earlier patches from mconnor

obsolete, but this patch has landed (on the trunk) with the patch for bug #318168
Attachment #224502 - Attachment is obsolete: true
the new events have landed with the patch in #318168 (on the trunk at least).
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: SWAG: 1d → SWAG: 1d 181b1+
marking fixed1.8.1 by bug 318168
Keywords: fixed1.8.1
Whiteboard: SWAG: 1d 181b1+
Probably a bit late now, but might it make sense to have a cancelable TabClose event, followed by a non-cancelable TabUnload event as (AFAIK) happens for windows?
You need to log in before you can comment on or make changes to this bug.