Closed Bug 103834 Opened 23 years ago Closed 22 years ago

Personal Toolbar is missing "Open in New Tab" on the context menu.

Categories

(SeaMonkey :: Tabbed Browser, enhancement, P3)

Product:
SeaMonkey
Component:
Tabbed Browser
Type:
enhancement

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
Future

People

(Reporter: ohrn, Assigned: bugs)

References

Details

Attachments

(1 file, 3 obsolete files)

updated per comments
5.36 KB, patch
jag+mozilla
: review+
jag+mozilla
: superreview+
Details | Diff | Splinter Review 
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.4+) Gecko/20011007
BuildID:    20011007

The new tabbed browser inteface is great, here's another one for the TODO list. :)

Personal Toolbar is missing an "Open in New Tab" option on the context menu and
it would also be nice if middle-clicking a link on the toolbar opens in a new tab.
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 98 → All
Hardware: PC → All
*** Bug 104370 has been marked as a duplicate of this bug. ***
Another really great feature would be being able to open links in new tabs with
ctrl-middle-mouse-button or shift-middle-mouse-button.
or maybe make the "Open in New Window" or "Open In New Tab" user configurable as
the default action  for the middle mouse button?
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.0.1
*** Bug 107928 has been marked as a duplicate of this bug. ***
spam: set your filter for "SeverusSnape" to avoid the influx of bugmail

changing QA contact of open tabbed browser bugs from blake to me. if this bug
requires a reassignment, however, feel free to change it!
QA Contact: blakeross → sairuh
Can we make this RFE a bit more generic by having all bookmarks (whether they
are in the menu bar or on the button in the personal toolbar) that are clicked
with the middle button open in a new tab?
QA Contact: sairuh → claudius
Reassigning to new component owner.
Assignee: hyatt → jaggernaut
Status: ASSIGNED → NEW
-> ben, p3, future
Assignee: jaggernaut → ben
Priority: -- → P3
Target Milestone: mozilla1.0.1 → Future
*** Bug 113298 has been marked as a duplicate of this bug. ***
*** Bug 127436 has been marked as a duplicate of this bug. ***
*** Bug 127638 has been marked as a duplicate of this bug. ***
*** Bug 128204 has been marked as a duplicate of this bug. ***
I'm including my first mozilla patch.  It adds "Open in New Tab" to the context
menu of the Personal Toolbar.

At this point I've only enabled the functionality for 
the type http://home.netscape.com/NC-rdf#Bookmark not
http://home.netscape.com/NC-rdf#IEFavorite or 
http://home.netscape.com/NC-rdf#FileSystemObject because I don't know if they
need special handling and I don't know how to place eithier of these items onto
the toolbar.

Why does only the openNewTabWith(url) function in contentAreaUtils.js do a
security check on the url (via the urlSecurityCheck function)?  No other places
in the code seem to do any sort of check when opening an item in a new tab. 
Should I place a simliar check in my code?
Adding patch keyword.
Keywords: patch
Whiteboard: Needs review
Blocks: 123569
Keywords: review
Ben, Chris: Will this bug fix mouse control of open in new tab (so bug 134816 is
dupe) or will be mouse control solved separately (in bug 134816?)?
*** Bug 134816 has been marked as a duplicate of this bug. ***
*** Bug 135684 has been marked as a duplicate of this bug. ***
*** Bug 139853 has been marked as a duplicate of this bug. ***
You should be able to control-click a link on the personal toolbar, so that it
opens up a new tab (for those of us w/o a middle mouse button).
Justin thats covered by the patch I'm working on for bug 59132.
*** Bug 145682 has been marked as a duplicate of this bug. ***
*** Bug 148045 has been marked as a duplicate of this bug. ***
Comment on attachment 76375 [details] [diff] [review]
Personal Toolbar Patch 1.0

>+        return (getBrowser && getBrowser() && getBrowser().localName == "tabbrowser");

May want to check |'localName' in getBrowser()| as well.
May also want to call getBrowser() just once and cache the result....

>+  open: function (aEvent, aRDFNode, aOptions) 

"aTarget" is more meaningful than "aOptions" given the expected values....

>+    else if (aOptions=="new_window")

spaces around the "==" ?

>+    else if (aOptions=="new_tab")

And again?

>+      if (getBrowser && getBrowser() && getBrowser().localName == "tabbrowser")
>+      {

The prevailing style fot this file is

if (something) {
  // code
}

Please follow that?

>+        var theTab, loadInBackground;
>+	theTab = getBrowser().addTab(urlValue); // open link in a new tab

weird indentation.....

>+        loadInBackground = pref.getBoolPref("browser.tabs.loadInBackground");

Check that "pref" is non-null before calling getBoolPref?  You may also want to
put
the whole pref access in a try/catch in case the pref is just not set at all.
Attached patch updated patch (obsolete) — — Splinter Review 
Updated with all of your recommendations.   In addition "Open in New Tab" is
now above "Open in New Window" to match the ordering of the link context menu.

*Open
*Open in New Tab
*Open in New Window
------------------
*** Bug 151099 has been marked as a duplicate of this bug. ***
*** Bug 152187 has been marked as a duplicate of this bug. ***
*** Bug 152510 has been marked as a duplicate of this bug. ***
*** Bug 153614 has been marked as a duplicate of this bug. ***
Any movement on this? (The patch will need to be adjusted to put "Open in New
Tab" below "Open in New Window".)
Attached patch updated (obsolete) — — Splinter Review 
Changed order back to

*Open
*Open in New Window
*Open in New Tab
------------------

Please review.
Has this been approved by UI? imo anyone who wants to open a bookmark in a new 
tab is fully capable of dragging the bookmark to empty space in the tab area.

We need this feature like we need a 100 item context menu.
Whiteboard: Needs review
The issue here is about consistency. Right clicking on a link in a page 
(Windows) brings up the New Tab ability, so to be consistent, it should do the 
same in the toolbar.

A menu item is more intuitive because it doesn't require the normal user to know
about that "trick".  I understand not wanting the menu to get too large, but I
believe this option will be used much more than some of the current menu items.
the toolbar is owned by bookmarks. if we put this silly menu in, then people 
will want it for bookmarks, and for url proxies, and for mailnews, and then 
they'll ask us to add it to windows explorer (we are technically capable of 
doing this).
Timeless: Open in new tab in MailNews was AFAIK wontfixed because it is another
product than Browser. People already want same item for bookmarks (bug 117449)
and IMHO it's correct, because bookmarks is same supporting tool for browsing as
Personal Toolbar. If you like browsing using new windows, you already satisfied,
but other people like browsing using new tabs. 

Chris Nebergall: some patches are obsoled now, could you mark them?
Attachment #76375 - Attachment is obsolete: true
Attachment #85718 - Attachment is obsolete: true
Timeless:  Most people already want to use tabs everywhere.  If you don't like
tabs for some reason, write a pref to turn them off competely, and I'll add code
to this patch to obey it.

Adam Hauner: I marked several old patches obselete. I would have done it
earlier, but since the bug is not assigned to me, I didn't think bugzilla would
let me.
I don't think dragging the bookmarks to the tab area is an adequate substitute
for two reasons.

1. You have to know the trick. I sure didn't know it, and I doubt many people do.
2. There doesn't seem to be a tab area to drop it in if only one page is up now.
edit>preferences>tabbed browser
change the pref to always show tabbar.

I'm aware of the bookmarks bug, and if it gets fixed then someone will demand 
that history add the same feature. It doesn't belong in any of them.

what we should probably do is give a drop target indicator when you try to drag 
a bookmark across the tabbar.
Timeless: When your tabbar is full of already open tabs, you can't drag a link
onto a *free* tabspace. (and I also didn't know about that trick) I'm looking
forward for a newtab items in context menus for PT/bookmarks/etc and same
ctrl/middleclick behaviour )

If there are "Open In New Window" items already in all links/bookmarks/whatever
context menus but "Open In New Tab" only in links in a page, that's inconsistent
IMHO. Why it doesn't belong there? I know it's *not* a link, but I think that
joe the user sees it as a "something *like a link*" (when I told my girlfriend
about "middle-click on a link opens in a new tab" she liked it and several
minutes later she was trying to use it on a sidebar bookmark ...)
you can drop it on the new tab icon.

as i said in the other bug i'm willing to suggest open in new window be removed 
from the context menu.
Why not remove the current "open" option and replace it with this one? Normal
links don't have an "open" item in their context menus either; you have to click
them to open. Everyone knows that. Everyone understands that. How are links on
the Personal Toolbar any different? They underline when you hover over them,
there can be no confusion as to what they are. The Open option is totally
unnecessary. Not having the New tab option, however, is inconsistent and
annoying. I should *never* be required to open a new window. I expect to be able
to open *any* link in the program, anywhere, a new window, as well as in a new
tab. A link shouldn't be different from another one, just because it's in a
slightly different place. New tab is just as essential for any kind of link as
having Properties for every link (and who looks at that anyway? I assure you
that a lot more people will want the New Tab than the Properties option for a
link; hence the 15 duplicate reports and 13 votes for this bug).
So if clutter bothers you, maybe just remove that weird and inconsistent "open"
option, but please put this in for consistencies sake, as well as a much nicer
user experience for those of us who like tabs.
> Normal links don't have an "open" item in their context menus either

This is arguably a bug.  In fact it's definitely filed as one.

> They underline when you hover over them,

And there is a bug to make them not do that.



I think this extra option would be really worthwhile, whether it is from a
"philosophical" point of view or not. The number of people CC:'d to this bug
attests that I'm not the only one that would use it all the time - I don't want
a "tab" bar when I'm only viewing one page in a window, and I didn't know the
trick either.
I think we should stop this "development view" concerning this bug - the user
doesn't care whether a Bookmark is something different from a link as:

- from user point of view _both_ entities (bookmark and link) take him to a
different page than the one he's currently viewing - and hence ...

- for both items it should be possible to open this new location in either the
same window or another window or in another tab

so I don't understand all the discussion related to "differences between links
and bookmarks" - of course there are differences, but in this special case the
user doesn't care abou them so they should _not_ prevent us from implementing a
useful and comfortable "feature" which is obviously requested by so many folks
out there...
For what it's worth, I do think that the context menu on bookmarks in the
personal toolbar should have an "open in new tab" menu item, since the personal
toolbar is part of the browser window the tab would be opened in.

This doesn't imply that we should add similar menu items to e.g. the history
window or the bookmarks management window or their sidebar counterparts, though
I don't see a problem with that.

About the patch, it looks okay-ish. The test |if (getBrowser)| should be |if
("getBrowser" in window)|. Instead of |tmpBrowser| just say |browser| or
|tabbrowser|.

Ben, could you take a quick peek at the patch and see if you like it?
*** Bug 158780 has been marked as a duplicate of this bug. ***
Keywords: mozilla1.2
*** Bug 166091 has been marked as a duplicate of this bug. ***
I have no objections to the motivation of this patch - it simply extends
capabilities available to page links to toolbar bookmarks. I do have a question
- does this patch also make it so this item is added to bookmarks in the sidebar
and management window?

For parity, shouldn't we also make it so that Ctrl/middle click on these toolbar
items causes the link to open in a new tab?
This patch only adds "open in new tab" to items on the Personal Toolbar.  I
don't mind writing patches for each of the other locations you mentioned, but I
think other bugs should cover them.  The main reason being I think the PT is the
most important (read useful) places for the menu item and I feared resistance to
landing this patch would have increased exponentially if I covered all the
locations in one patch and it would never be landed.

My patch for bug 59132 covers modified and middle clicks on the Personal
Toolbar.  Again, that bug has been waiting ~2 months for review and any help you
could give would be appreciated.
+        return (tmpBrowser && 'localName' in tmpBrowser && tmpBrowser.localName
== "tabbrowser");

also, stick to using double quotes (") rather than single quotes (')

fix that and jag's stuff and r/sr=ben@netscape.com
>Instead of |tmpBrowser| just say |browser| or |tabbrowser|.
If you just meant change the variable name to |browser| that's what I did.  I
also made the other changes mentioned.

After the official reviews are marked, can someone check this in?  I don't have
the necessary permissions.
Attachment #90235 - Attachment is obsolete: true
Comment on attachment 97533 [details] [diff] [review]
updated per comments

Marking r=jag, sr=ben
Attachment #97533 - Flags: superreview+
Attachment #97533 - Flags: review+
Patch has been checked in.  Could someone close this bug? Again, I don't have
the necessary permissions.  RESOLVED->FIXED
this is FIXED.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
VERIFIED
Status: RESOLVED → VERIFIED
This isn't like 100% fixed really since you can't "open in new tab" links which
are inside folders on the personal menu.
Please, extend the context menu to *all* bookmark inside the dropdown folder
(and subfolders as well) of personal toolbar, bookmark handling seem mutilated
to me without this simple feature. There is no reason for users to perceive
differences between the bookmark tree in sidebar and bookmark menu/tree in
personal bookmark.
I think you guys want bug 176880. I assume as part of that bug they would
support all of the current menu items on the new PT folder's context menus.  
yes, bug 176880 and also bug 50504 are very similar, they are dupe of each other
in my opinion, they express a long awaited user needs of a consistent bookmark
handling, should have top priority for 1.3a release.
You must mean 1.3b.  Because something that would be top priority for 1.3a
should have been top priority a month ago.

Also, a hint.  If the developers' priorities and yours don't coincide, telling
them their priorities are wrong is not likely to change anything other than to
make them stop listening to everything you say.
Product: Core → SeaMonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: