Closed Bug 665678 Opened 13 years ago Closed 13 years ago

Open Add-ons Manager, Data Manager etc. according to Link Behavior preferences (i.e. in a window if the user chose so)

Categories

(SeaMonkey :: UI Design, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.7

People

(Reporter: dsmutil, Assigned: InvisibleSmiley)

Details

Attachments

(1 file)

Opening the new Data Manager, Cookie Manager, Popup Manager, or Image Permissions opens as a new tab, instead as a separate window such as the Preferences or Bookmark Manager, or how all of these used to work. First, I think this is awkward since you normally don't expect a new tab for system settings, especially if you don't normally use tabs. Second, this should probably, as a minimum, honor Preferences/Browser/Link Behavior so it's not confusing to users that don't use tabs. Thanks.
According to the below, it's at least Add-ons Manager, Data Manager, and possibly Tabs From Other Computers (but I'd say the correct thing in the latter case would be to do as with Home, i.e. replace the current page). Adjusting summary and moving.

http://mxr.mozilla.org/comm-central/search?string=switchToTabHavingURI&find=suite&tree=comm-central

The interesting part therein is probably this:
  openUILinkIn(aURI.spec, "tabfocused");
Component: Passwords & Permissions → UI Design
OS: Windows XP → All
QA Contact: privacy → ui-design
Hardware: x86 → All
Summary: Data Manager opens as tab instead of a window → Open Add-ons Manager, Data Manager etc. according to Link Behavior preferences (i.e. in a window if the user chose so)
Version: SeaMonkey 2.1 Branch → Trunk
switchToTabHavingURI currently takes for granted that it's always tabs are are wanted, I guess we might want to look into changing that.
Not taking yet, I need some feedback first. Could it really be this easy? I found that it did what it should, including cases like Manage Stored Cookies or Manage Image Permissions which open the DM with a certain view.

You'll see that the code is ripped from loadAddSearchEngines. Would it make sense to let that one call switchToTabHavingURI, too?

Philip, is changing switchToTabHavingURI OK from an FF/add-ons compat POV?
Attachment #563858 - Flags: review?(neil)
Attachment #563858 - Flags: feedback?(philip.chee)
Comment on attachment 563858 [details] [diff] [review]
patch [Checkin: comment 8]

> Not taking yet, I need some feedback first. Could it really be this easy? I
> found that it did what it should, including cases like Manage Stored Cookies
> or Manage Image Permissions which open the DM with a certain view.

1. Looks logical. I assume you tested all callers.

> You'll see that the code is ripped from loadAddSearchEngines. Would it make
> sense to let that one call switchToTabHavingURI, too?

Sounds reasonable, but I'd check to see if you don't regress anything.

> Philip, is changing switchToTabHavingURI OK from an FF/add-ons compat POV?

It should not be a problem as the "new window" case opens the manager in a _tab_ in a new tabbed browser window anyway.
Attachment #563858 - Flags: feedback?(philip.chee) → feedback+
Assignee: nobody → jh
Status: NEW → ASSIGNED
Comment on attachment 563858 [details] [diff] [review]
patch [Checkin: comment 8]

Looks reasonable, but I think we should be using open_external rather than open_newwindow which should be solely for diverting clicked links. (Sorry for not spotting it in the search engine code before.)
Attachment #563858 - Flags: review?(neil) → review+
IMHO, using open_external for anything coming from _inside_ SeaMonkey was an error all along, but unfortunately it's where we ended up. :(
(In reply to neil@parkwaycc.co.uk from comment #5)
> I think we should be using open_external rather than
> open_newwindow which should be solely for diverting clicked links.

Well, it's choosing between bad and worse. The one is meant to be used only for links, the other only for things opened from external applications (cf. Preferences/Browser/Link Behavior wording). The only clean solution would be to introduce yet another pref which would then have to be added to the Link Behavior pref panel. But I guess that's not worth it. I have no particular preference (heh) which rule to break, so why not go with your suggestion.

> (Sorry for not spotting it in the search engine code before.)

Actually this case supports your rule: The trigger in the search engine manager is a link which opens a web page. Thus it should stay open_newwindow, which is why I'll keep the loadAddSearchEngines as it is.
Comment on attachment 563858 [details] [diff] [review]
patch [Checkin: comment 8]

http://hg.mozilla.org/comm-central/rev/5d28210323c4
with pref = browser.link.open_external
Attachment #563858 - Attachment description: patch → patch [Checkin: comment 8]
I think this should bake a bit on trunk, then we can think about having it on Aurora, too.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.7
IMO, logically this should have used the browser.link.open_newwindow as b.l.open_external is for "links from other applications" and not something coming from inside SM itself. I don't see why about:addons or about:data are not "links" in your eyes. :) Besides, both prefs are about links so that shouldn't matter. It's the link origin that determines the pref to use.
(In reply to Teemu Mannermaa (:wicked) from comment #10)
> IMO, logically this should have used the browser.link.open_newwindow as
> b.l.open_external is for "links from other applications" and not something
> coming from inside SM itself.

Please read comment 7. Thanks.
Just seeing this on a SM 2.7.1 installation where I have browser.link.open_external coincidentally set to "1" (corresponding to "The current tab/window"). However, both Data Manager and Add-ons Manager open in a /new/ window rather than in the currently active one.

Is that the intended result for this setting?

"A new tab in the current window" shows the default behavior as before.
You need to log in before you can comment on or make changes to this bug.