100706 - Implement <tabbrowser> widget (for use by clients like MultiZilla)

Status: VERIFIED FIXED

(SeaMonkey :: Tabbed Browser, defect)

Description

[David Hyatt]

This bug covers making an XBL widget for tabbed browsing.  The idea is to make a
reusable version of the Multizilla paradigm for use by all.

Comment 1

[David Hyatt]

Attachment: Initial stab. Very rough.

Comment 2

[HJ]

Here's a link http://multizilla.mozdev.org/index.org to the MultiZilla project ;)

Comment 3

[HJ]

You fool, make that: http://multizilla.mozdev.org/index.html

Comment 4

[Håkan Waara]

Would this be a pref? Would this be code not used by default?

Please elaborate on your plans to implement this.

Comment 5

[David Hyatt]

Attachment: Fix a bug in tab removal. Rename browser domWindow prop to contentWindow to match what jst just did for html iframes.

Comment 6

[Brendan Eich [:brendan]]

Comment on attachment 50350 [details] [diff] [review]
Fix a bug in tab removal.  Rename browser domWindow prop to contentWindow to match what jst just did for html iframes.

sr=[e-mail]

Comment 7

[jag (Peter Annema)]

Comment on attachment 50350 [details] [diff] [review]
Fix a bug in tab removal.  Rename browser domWindow prop to contentWindow to match what jst just did for html iframes.

r=jag

Comment 8

[David Hyatt]

The basic widget is in.  Time to start filing separate bugs for issues.  Closing
this one out.

(CTRL+T is the super-secret keystroke to bring up extra tabs.)

Comment 9

[David Hyatt]

Going to reopen this and use it to track work.

Comment 10

[David Hyatt]

Attachment: Remove tab progress listeners when tabs go away. Fix tab tooltips.

Comment 11

[Håkan Waara]

Just wondering, aren't new files supposed to be checked in triple-licensed?
browser.css was checked in with MPL 1.1...

Comment 12

[Brendan Eich [:brendan]]

hyatt, here's the new boilerplate for various flavors of source:
http://www.mozilla.org/MPL/boilerplate-1.1/

Nit: no need to test this.mTabListeners.length > 0 in the destructor, the loop
test suffices.

Does the tooltip need to be firstChild?  Or is that just convention?  Educate
me, I'm ignorant.

/be

Comment 13

[Jason Kersey]

hyatt, is bug 101131 caused by this?

Comment 14

[David Hyatt]

Jason, nope.  

Brendan, there's an interesting problem with the placeholder frames for popups
(e.g., tooltips) being stretched by the stack and then taking mouse events
(since the placeholder is on top).  I'm working around the problem without
fixing the bug, since I need to think about how to ensure that box never sizes a
placeholder to anything other than (0,0).

I'm thinking of hacking into placeholder frame's SetBounds function and not
allowing the rect to be anything other than 0 width and 0 height.

Comment 15

[jag (Peter Annema)]

Attachment: mirror addProgressListener change in navigator.js for removeProgressListener

Comment 16

[Blake Ross]

r/sr=blake

Comment 17

[David Hyatt]

jag, sr=hyatt.

Comment 18

[jag (Peter Annema)]

Comment on attachment 50371 [details] [diff] [review]
Remove tab progress listeners when tabs go away.  Fix tab tooltips.

Fix the length check nit brendan pointed out and you'll have r=jag

Comment 19

[David Hyatt]

Attachment: Patch that fixes up removal of listeners and tooltips, and also adds support for loading status. Also fixes bugs 98109 and 101093.

Comment 20

[David Hyatt]

Please r/sr this patch here, or go over to 98109 and 101093 for more info on
those specific issues that are also contained in this patch.

Comment 21

[Johnny Stenback (:jst)]

Comment on attachment 50432 [details] [diff] [review]
Patch that fixes up removal of listeners and tooltips, and also adds support for loading status.  Also fixes bugs 98109 and 101093.

r/sr=jst

Comment 22

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 50432 [details] [diff] [review]
Patch that fixes up removal of listeners and tooltips, and also adds support for loading status.  Also fixes bugs 98109 and 101093.

r=[e-mail] for the js/xbl stuff.

Comment 23

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 50432 [details] [diff] [review]
Patch that fixes up removal of listeners and tooltips, and also adds support for loading status.  Also fixes bugs 98109 and 101093.

r=[e-mail] for the js/xbl stuff.

Comment 24

[David Hyatt]

Ok, clean slate.  Everything has been checked in.

Comment 25

[Jens-Uwe]

hmm, which are the hotkeys (new tab, cycling through tabs and so on), and how
will they be promoted, that the enduser will find this function?

Comment 26

[David Hyatt]

Attachment: Ensure window title updates properly on a tab switch. Add support for addProgress and removeProgress listener apis to <browser> so that we can ensure <tabbrowser> and <browser> work the same.

Comment 27

[David Hyatt]

The UI is deliberately hidden right now, until it has been reviewed/tested. 
CTRL+TAB and CTRL+SHIFT+TAB cycle between tabs in a tab widget (these are the
standard Win32 keys for tabs).  CTRL+T opens a new tab.

Comment 28

[Jason Kersey]

Attachment: Close image - black X transparent BG

Comment 29

[David Hyatt]

Attachment: Whole slew of fixes, support for pref-based opening of new tabs for urlbar and middle click.

Comment 30

[David Hyatt]

Ok, this newest patch is ready for r/sr.

Comment 31

[jag (Peter Annema)]

Comment on attachment 50486 [details] [diff] [review]
Whole slew of fixes, support for pref-based opening of new tabs for urlbar and middle click.

>Index: themes/modern/global/browser.css
>===================================================================
>RCS file: /cvsroot/mozilla/themes/modern/global/browser.css,v
>retrieving revision 1.1
>diff -u -r1.1 browser.css
>--- browser.css	2001/09/23 09:33:15	1.1
>+++ browser.css	2001/09/24 00:53:18
>@@ -4,3 +4,9 @@
> tab[busy] {
>   list-style-image: url("chrome://communicator/skin/icons/loading.gif");
> }
>+
>+.tabbrowser-closebox {
>+  list-style-image: url("chrome://global/skin/icons/closebox.gif");
>+  margin-bottom: 3px;
>+  margin-right: 2px;
>+}
>\ No newline at end of file

^^^^^^^^^^^^^^^^ Nit.

>Index: xpfe/browser/resources/content/navigator.js
>===================================================================
>RCS file: /cvsroot/mozilla/xpfe/browser/resources/content/navigator.js,v
>retrieving revision 1.366
>diff -u -r1.366 navigator.js
>--- navigator.js	2001/09/23 09:29:58	1.366
>+++ navigator.js	2001/09/24 00:53:34
>@@ -898,7 +898,13 @@
>   if (url.match(/^view-source:/)) {
>     BrowserViewSourceOfURL(url.replace(/^view-source:/, ""), null);
>   } else {
>-    loadURI(getShortcutOrURI(url));
>+    if (pref && pref.GetBoolPref("browser.tabs.opentabfor.urlbar") && getBrowser &&
>+        getBrowser() && getBrowser().localName == "tabbrowser") {

This should suffice:

  if (pref.GetBoolPref("browser.tabs.opentabfor.urlbar") &&
      getBrowser().localname == "tabbrowser")

Since you're in navigator.js and will know each to exist, unlike in contentAreaClick.js (ew!).

>Index: xpfe/global/resources/content/bindings/browser.xml
>===================================================================
>RCS file: /cvsroot/mozilla/xpfe/global/resources/content/bindings/browser.xml,v
>retrieving revision 1.4
>diff -u -r1.4 browser.xml
>--- browser.xml	2001/09/23 09:30:02	1.4
>+++ browser.xml	2001/09/24 00:53:42
>@@ -47,15 +47,15 @@
>     <content>
>       <xul:tabbox onselect="this.parentNode.updateCurrentBrowser();">
>         <xul:stack collapsed="true" tooltip="_child">
>-          <xul:tooltip onpopupshowing="event.preventBubble(); if (document.tooltipNode.label) { this.childNodes[0].value = document.tooltipNode.label; return true; } return false;">
>+          <xul:tooltip onpopupshowing="event.preventBubble(); if (document.tooltipNode.getAttribute('label')) { this.childNodes[0].value = document.tooltipNode.getAttribute('label'); return true; } return false;">

???

>             <xul:label style="margin:0px; padding:0px;" value="Dummy"/>
>           </xul:tooltip>
>-          <xul:tabs flex="1">
>-            <xul:tab flex="1" maxwidth="150" class="tabbrowser-tab" label="Blank" crop="center"/>
>+          <xul:tabs style="padding-left:1px; padding-top: 1px;" flex="1">
>+            <xul:tab flex="1" maxwidth="150" class="tabbrowser-tab" label="(Untitled)" crop="center"/>

Make a note that this needs to be tuned into an entity.

>@@ -142,7 +143,8 @@
>                        aStateFlags & nsIWebProgressListener.STATE_IS_NETWORK) {
>                 this.mTab.removeAttribute("busy");
>                 if (this.mTab.label == "Loading...")
>-                  this.mTab.label = "Done";
>+                  this.mTab.label = "(Untitled)";
>+                

Same for this.

>@@ -318,7 +347,6 @@
>-            b.setAttribute("src", aURI);
...
>+            b.loadURI(aURI, nsIWebNavigation.LOAD_FLAGS_NONE);

???

Comment 32

[David Hyatt]

Addressing your two ???

loadURI has to be used over the src attribute because loadURI knows how to fix 
up a URL from the URL bar, e.g..  The src attribute does not.  Plus it's faster 
to use loadURI anyway, since you don't waste time mucking around with a content 
model.

The tooltip listener on the tabbrowser looks for label attributes and since the 
close box is an image, it doesn't support .label, so I switched it.

Comment 33

[jag (Peter Annema)]

Comment on attachment 50486 [details] [diff] [review]
Whole slew of fixes, support for pref-based opening of new tabs for urlbar and middle click.

r=me then

Comment 34

[Topics Man]

> The UI is deliberately hidden right now, until it has been reviewed/tested. 
> CTRL+TAB and CTRL+SHIFT+TAB cycle between tabs in a tab widget (these are the
> standard Win32 keys for tabs).  CTRL+T opens a new tab.

CTRL+TAB and CTRL+SHIFT+TAB dont work on the last nightly (ID 2001092308), 
obviously after some tabs have been opened with CTRL+T.

Also in vague conformance to the Win32 conventions, will CTRL+F4 close a tab?

Cheers,
Chris

Comment 35

[Blake Ross]

sr=blake with the prerequisite discussed on IRC.

Comment 36

[David Hyatt]

Ok, state cleared again.  Everything checked in.

Comment 37

[[not reading bugmail]]

I just wanna comment about this... it actually works, this is really a neat
thing. I know Opera has the ability to do tabs within the application shell, and
now I dont have to download Multizilla which is flaky at best right now with all
its add on features.. Although having Multizilla and this tab feature work for
Netscape 6.1 or 6.2+ would really be a treat over what IE can provide, and maybe
just maybe gain some market share :), I really hate multiple browser windows: 
it is so much cleaner on my desktop and taskbar, easier to get to another window
quickly if your taskbar is a mess.  I'd say this is a plus in the end user
experience!

In refering to a post in the newsgroups about access Xul documents for mail and
composer in the window, would it be possible then to have the thread for the
whole "composer app" or "mail app" or "address book" spawn into a tab?
news thread refered to: netscape.public.mozilla.general - "a mozilla trick" 9/18

- Anyway very cool!  I like it.  Ctrl-T works on 20012308. no crashes or
problems either for me on W2k.

Comment 38

[[not reading bugmail]]

[RFE] - have a link open in new tab by default with "tabs feature on" would be
cool too!

-opening too many tabs needs a scrolling feature between tabs for this to really
be useful for many open tabs. i.e. try opening several tabs: it extends the
browser window all goofy.. not a good thing.

Comment 39

[Jens-Uwe]

hyatt:
crtl+tab and crtl+shift+tab do not work with KDE2.2. 
I also miss a function to remove tabs after they are created.

Comment 40

[jkng]

The page title (above "File", "edit", etc menu navigation) does not change once
tabs are opened. It should also change depending on which tab is focused. 

Comment 41

[Anthony Baxter]

neato. things I noticed: 
opening a new tab gives me a blank screen - not about:blank, my homepage
(which is a file:///) or something similar. Shouldn't it follow the standard
pref for new windows?
Window title gets left at what it was when you first turned on the tabbing.
When you turn off tabbing (back to the single window) it doesn't get reset to
the current window's title - you have to go to a new link. 
If I have multiple tabs open, and I select one of the inactive ones from 
the toplevel 'Tasks' menu, it doesn't bring them to the front.

On Linux, it's scary how much faster this is than to bring up a new window
and switch back and forth between windows.

Comment 42

[jlarsen]

[RFE] Open in New Tab 
in context menu, when tabs are enabled :) should be first item on list.

Comment 43

[unapersson]

This is a wonderful new feature. All we need now is a preference to force
window.open to create just a new tab instead. That way we can have multiple 
windows as usual but keep frenetic window openers trapped within a single window.

Comment 44

[dales]

I do like the tabbed interface.

There is one problem I have noted with the interface ... it might be counted a
regression (at least from what happens when you are in single-window mode)
and/or a RFE:

Open multiple tabs, 
take one of them to a long page, 
scroll down the page.  

Now minimize the browser and then restore ... 
SCROLL position is no longer remembered, you come back at the top of the page.  

It behaves the same, if the scrolled tab is not the currently active tab when
you minimize the browser.

I would like to see scroll-position memory in the tabbed panes.  

Dale

Comment 45

[Adam D. Moss]

This is truly wonderful.

One buglette and one RFE:

* when switching between tabs with the mouse, keyboard
focus remains on last-focused tab (ie. page A has
focus, switch to page B, try to use arrow keys to
scroll around -- no observable effect, but you're
actually scrolling the hidden page A around)

* a pref to open middle-mouse-button-click links in a
new tab instead of a new window would make my life
complete.  Or perhaps more comprehensively have the
pref make tabs altogether usurp new-browser-window
semantics so CTRL-N and similar also simply pop into
new tabs.

Comment 46

[jkng]

RFE: 

* When tabs are activated, ctrl+w closes the active tab not the browser window. 

* Ctrl+left-mouse-click and middle-mouse-click open link in a new tab when
activated. 

* These keyboard/mouse shortcuts should switch back to their default behaviors
when tabs are not turned on. 

Comment 47

[Dave]

Just noticed that alt+right click on a tab pulls up my window manager's --
enlightenment's -- properties menu for the mozilla window (i.e. selecting
"close" on the menu closes the whole browser window). Is this a feature? :-)

Comment 48

[Dave]

Yes, it appears to be an Enlightenment feature which is true for all windows...
never mind.

Comment 49

[Dave]

Filed bug 101554 for tab related crash.

Comment 50

[David Hyatt]

set browser.tabs.opentabfor.middleclick to true.  I implemented that already.

Comment 51

[Joni K]

The keyboard shortcuts (ctrl+tab and ctrl+shift+tab) don't work in KDE because
they are used to switch between virtual desktops.

Suggestion: how about using Shift + left/right arrow for this? 
KDE's Konsole uses those keys to traverse tabs and IMHO it's really convenient.

Comment 52

[Pratik]

You can use Ctrl-Tab and Ctrl-Shift-Tab in KDE if you disable them in Control
Centre (for Walk through Desktop list). One bug though, the tabs are not getting
cycled, meaning, that pressing Ctrl-Tab after the last Tab does not move it to
the first tab.

I think Ctrl-Tab and Ctrl-Shift-Tab are good key choices. What I'd really love
is some kind of a UI in preferences so that people who want to, can change these
key bindings. At the very least a pref in prefs.js would be nice. 

I don't think we can decide on Shift-arrow key or something random like that.

Comment 53

[Adam D. Moss]

's funny, the browser.tabs.opentabfor.* prefs just
don't seem to be working here (linux, cvs-head).

Comment 54

[HJ]

Adam, all of them?

Comment 55

[Adam D. Moss]

.middleclick and .bookmarks at least (can't test
the others for quite a while since I'm rebuilding)

Comment 56

[Adam D. Moss]

Ah, one more checkout and rebuild later and things are
better!  Sorry for the false alarm.

Comment 57

[HJ]

David, the link coloring stuff (presshell) doesn't work. 

Comment 58

[Ben Ruppel]

Is this alt-T thing still supposed to be a secret?  I could imagine a huge,
positive, user response if word got out (say, in the newsgroups).

Comment 59

[unapersson]

Just discovered a new bug: there are now noticeable flickers with pages using
mouseover images if you view them in the tabbed interface. There are no problems
viewing them in a normal Mozilla window.

Comment 60

[Pratik]

Ben,

I don't think its that big a secret. Asa wrote on Mozillazine 

'If you didn't notice over the weekend you should grab today's builds and press
CTRL+T (the temporary initialization of the new tabs toolbar).'

So I think its safe to say, Spread the joy. Let everyone know about this
wonderful new feature.

Comment 61

[Jeremy Sanders]

Just a pedantic, philosophical point. Given that the address (and button) bar is
associated with the current tab, shouldn't the address and button bar be on the tab?

This is a bit of a picky thing, but the tab metaphor suggests that changing the
tab should only change which tab is viewed.

Maybe as this is an "advanced feature", it doen't matter.

Comment 62

[PeS]

I noticed coredumps with Gecko/20010926 when a lot of tabs is open and some is
loading...i try some to close and...ooops :-(
But the other think I'd like to say: when the window is too small to fit all
tabs, the main window is greater than view, so the close button and mozilla logo
etc is out of window...maybe resizing tab size or making second row of tabs
could solve it.

Comment 63

[Blake Ross]

This bug is not a forum for general issues with tabbrowser.  The fact that you
have to press ctrl+t to even access the feature should be a hint that it's not
yet ready for primetime, and thus it's somewhat futile to be discussing (already
known) problems at this point...

Comment 64

[PeS]

Ahh, ok, I'm sorry. Maybe next time write something about this at start of
comments? (I thought, that when CTRL-T is enabled, that bugzilla wants to know
bug reports etc...my mistake.) Keep up good work...

Comment 65

[Tom van Wietmarschen]

When switching tabs with CTRL-TAB and CTRL-SHIFT-TAB the pages are scrolled back
to the top. this does not happen when clicking the tabs. (build 2001092709) 

Comment 66

[Ben Ruppel]

funny, ctrl-tab doesn't switch through tabs in WinXP.  I'm pretty sure it
doesn't work in win2k either.

Comment 67

[Joni K]

You're right, ctrl-tab and ctrl-shift-tab don't work in w2k (using build
2001092809).

Comment 68

[Blake Ross]

-> tabbrowser

Comment 69

[David Hyatt]

*** Bug 59126 has been marked as a duplicate of this bug. ***

Comment 70

[David Gerard]

ctrl-tab and ctrl-shift-tab don't seem to work in NT4 either (all builds from when
this went in up to this one, 2001100403).

Comment 71

[Jonadab the Unsightly One (Nathan Eady)]

Ctrl-Tab and Ctrl-Shift-Tab don't switch between tabs in Win95 OSR2 either,
build 2001102309.  Not sure if it matters, clicking the tabs with the
mouse seems okay for now.  Maybe a menu item under View for selecting
which tab is active?

Re Jeremy Sanders' comment, I disagree.  I like the tab bar below
the toolbars, where it is now.  Yes, some of the toolbars (most
notably the location bar) display page-specific content that 
changes when you switch tabs, but others (e.g., the personal
toolbar, the preferences toolbar) display content that is not
page-specific at all, and in general the toolbars display 
browser features, not page content as such.  Even the address
in the location bar, though it relates to a specific page, is
nevertheless a feature of the browser, rather than a part of
the page per se.  In any case, bugs like 48926 and especially 
15144 are planning to fundamentally change the way toolbars are
arranged and place it more in the hands of the user anyway.

Comment 72

[Ben Ruppel]

Hyatt: ctrl-tab and crtl-shift-tab don't work in win32 platforms and linux with
KDE and the desktop switching turned off.  Should I file a bug?

Comment 73

[jag (Peter Annema)]

Ben, accel-tab and shift-accel-tab for switching tab browser tabs was recently
changed to (I think) accel-pgup and accel-pgdn (where accel is ctrl on
linux/windows, unless you've explicitely set it to alt).

Comment 74

[Peter Trudelle]

resolving as fixed

Comment 75

[John Morrison]

verified it's alive. use separate bugs for outstanding issues.

Comment 76

[David :Bienvenu]

a checkin for this bug removed the setting of global history, which breaks
history for me:

 nsCOMPtr<nsIDocShellHistory> dsHistory(do_QueryInterface(docShell));      
nsCOMPtr<nsIGlobalHistory> history(do_GetService(kCGlobalHistoryCID));    
if (dsHistory)                                                            
dsHistory->SetGlobalHistory(history);                                   

I can't find anyone who sets global history, except for the embedding code. The
doc shell no longer creates global history - it relies on SetGlobalHistory
getting called, and according to lxr, no one does that. Does anyone know what's
going on here?

Comment 77

[David Hyatt]

It's done in XBL for the <browser> element.