Closed Bug 1287642 Opened 3 years ago Closed 3 years ago

Open as dialog for Add-ons and Data Manager should no longer be associated with browser.link.open_external

Categories

(SeaMonkey :: Passwords & Permissions, defect)

SeaMonkey 2.45 Branch
defect
Not set

Tracking

(seamonkey2.45 fixed, seamonkey2.46 fixed, seamonkey2.47 fixed, seamonkey2.48 fixed)

RESOLVED FIXED
seamonkey2.48
Tracking Status
seamonkey2.45 --- fixed
seamonkey2.46 --- fixed
seamonkey2.47 --- fixed
seamonkey2.48 --- fixed

People

(Reporter: rsx11m.pub, Assigned: rsx11m.pub)

References

Details

Attachments

(1 file, 1 obsolete file)

1.24 KB, patch
philip.chee
: review+
iann_bugzilla
: approval-comm-aurora+
iann_bugzilla
: approval-comm-beta+
iann_bugzilla
: approval-comm-release+
Details | Diff | Splinter Review
+++ This bug was initially created as a clone of Bug #1255354 +++

STR:

 1. Set suite.manager.{addons,dataman}.openAsDialog to "true"
 2. Add-ons and Data Managers open as dialog as expected
 3. Set suite.manager.{addons,dataman}.openAsDialog to "true"
 4. Add-ons and Data Managers open in a browser tab as expected
 5. Set browser.link.open_external to "2"
    that's Edit > Preferences > Browser > Link Behavior
           - Open links passed from other applications in:
             - A new window
 6a. Add-ons and Data Managers open in a separate window = NOT expected
 6b. Add-ons and Data Managers should open in a browser tab as expected

Opening the Add-ons and Data Managers should no longer depend on that setting after bug 1255354.
Best guess: Check for "about:data" or "about:addons" URIs here when setting the "where" variable,
https://dxr.mozilla.org/comm-central/source/suite/common/utilityOverlay.js#1680
Assignee: nobody → rsx11m.pub
Status: NEW → ASSIGNED
Attached patch Easy fix (obsolete) — Splinter Review
I made it an array, thus it's easy to extend if we want to uncouple further special pages from the browser.link.open_external pref later.
Attachment #8772637 - Flags: review?(philip.chee)
Comment on attachment 8772637 [details] [diff] [review]
Easy fix

I think aOpenNew should ignore browser.link.open_external really.

Options:
(1) Completely remove the check for browser.link.open_external and just open tabfocused like Firefox.
(2) "about" pages are all internal (well ours are anyway. Firefox is weird).
> let where = aURI.schemeIs("about") || newWindowPref == kNewTab ? "tabfocused" : "window";

Your choice.
Attachment #8772637 - Flags: review?(philip.chee)
Blocks: 1287289
Attached patch Easier fix (v2)Splinter Review
Ok, I take it from your comment #3 that you prefer option (1) as the solution.
This obviously makes the code much easier with "tabfocus" being hard-wired.

I wasn't sure if switchToTabHavingURI() is called in a different context,
but all uses from suite/ are for "about:*" pages, thus should be safe.

I can't remove kNewTab as such given that it's needed elsewhere in this file.

We probably also want to have this on the branches, if you want to a+ those.
Attachment #8772637 - Attachment is obsolete: true
Attachment #8779535 - Flags: review?(philip.chee)
Comment on attachment 8779535 [details] [diff] [review]
Easier fix (v2)

looks good. a=me CLOSED TREE if necessary
Attachment #8779535 - Flags: review?(philip.chee) → review+
Comment on attachment 8779535 [details] [diff] [review]
Easier fix (v2)

Should be easy enough for the branches.

[Approval Request Comment]
Regression caused by (bug #): bug 1255354
User impact if declined: with non-default settings, open as tab may not work
Testing completed (on m-c, etc.): works on c-c
Risk to taking this patch (and alternatives if risky): low
String changes made by this patch: none
Attachment #8779535 - Flags: approval-comm-release?
Attachment #8779535 - Flags: approval-comm-beta?
Attachment #8779535 - Flags: approval-comm-aurora?
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.48
Comment on attachment 8779535 [details] [diff] [review]
Easier fix (v2)

a=me
Attachment #8779535 - Flags: approval-comm-release?
Attachment #8779535 - Flags: approval-comm-release+
Attachment #8779535 - Flags: approval-comm-beta?
Attachment #8779535 - Flags: approval-comm-beta+
Attachment #8779535 - Flags: approval-comm-aurora?
Attachment #8779535 - Flags: approval-comm-aurora+
You need to log in before you can comment on or make changes to this bug.