Bug 1817443 - remove openUILinkIn entirely and rename fromChrome, r=mossop,extension-reviewers,rpl

'fromChrome' really meant "force tabs to open in the foreground", so let's
rename it accordingly.

This removes the attempt to document arguments for openUILinkIn.
I'll add documentation back on the end of this stack, for openLinkIn, when
various bits are reorganized anyway.

Differential Revision: https://phabricator.services.mozilla.com/D170384
This commit is contained in:
Gijs Kruitbosch 2023-02-23 17:02:43 +00:00
parent 3e5d710873
commit fbd24413c4
11 changed files with 52 additions and 148 deletions

View File

@ -504,7 +504,7 @@
hidden="true"/>
<menuitem id="helpPolicySupport"
hidden="true"
oncommand="openUILinkIn(Services.policies.getSupportMenu().URL.href, 'tab', {triggeringPrincipal: Services.scriptSecurityManager.createNullPrincipal({})});"/>
oncommand="openTrustedLinkIn(Services.policies.getSupportMenu().URL.href, 'tab');"/>
</menupopup>
</menu>
</menubar>

View File

@ -33,8 +33,7 @@ async function test_bookmarks_toolbar_visibility({ newTabEnabled }) {
let newWindowOpened = BrowserTestUtils.domWindowOpened();
let beforeShown = TestUtils.topicObserved("browser-window-before-show");
let triggeringPrincipal = Services.scriptSecurityManager.getSystemPrincipal();
openUILinkIn(url, "window", { triggeringPrincipal });
openTrustedLinkIn(url, "window");
let newWin = await newWindowOpened;
let slowSiteLoaded = BrowserTestUtils.firstBrowserLoaded(newWin, false);

View File

@ -120,64 +120,12 @@ function whereToOpenLink(e, ignoreButton, ignoreAlt) {
return BrowserUtils.whereToOpenLink(e, ignoreButton, ignoreAlt);
}
/* openTrustedLinkIn will attempt to open the given URI using the SystemPrincipal
* as the trigeringPrincipal, unless a more specific Principal is provided.
*
* See openUILinkIn for a discussion of parameters
*/
function openTrustedLinkIn(url, where, aParams) {
var params = aParams;
if (!params) {
params = {};
}
if (!params.triggeringPrincipal) {
params.triggeringPrincipal = Services.scriptSecurityManager.getSystemPrincipal();
}
openUILinkIn(url, where, params);
function openTrustedLinkIn(url, where, params) {
URILoadingHelper.openTrustedLinkIn(window, url, where, params);
}
/* openWebLinkIn will attempt to open the given URI using the NullPrincipal
* as the triggeringPrincipal, unless a more specific Principal is provided.
*
* See openUILinkIn for a discussion of parameters
*/
function openWebLinkIn(url, where, params) {
if (!params) {
params = {};
}
if (!params.triggeringPrincipal) {
params.triggeringPrincipal = Services.scriptSecurityManager.createNullPrincipal(
{}
);
}
if (params.triggeringPrincipal.isSystemPrincipal) {
throw new Error(
"System principal should never be passed into openWebLinkIn()"
);
}
openUILinkIn(url, where, params);
}
function openUILinkIn(
url,
where,
aAllowThirdPartyFixup,
aPostData,
aReferrerInfo
) {
return URILoadingHelper.openUILinkIn(
window,
url,
where,
aAllowThirdPartyFixup,
aPostData,
aReferrerInfo
);
URILoadingHelper.openWebLinkIn(window, url, where, params);
}
function openLinkIn(url, where, params) {

View File

@ -233,7 +233,7 @@ class PlacesFeed {
const params = {
private: isPrivate,
targetBrowser: action._target.browser,
fromChrome: false, // This ensure we maintain user preference for how to open new tabs.
forceForeground: false, // This ensure we maintain user preference for how to open new tabs.
globalHistoryOptions: {
triggeringSponsoredURL: action.data.sponsored_tile_id
? action.data.url

View File

@ -282,7 +282,7 @@ describe("PlacesFeed", () => {
assert.equal(url, "https://foo.com");
assert.equal(where, "window");
assert.propertyVal(params, "private", false);
assert.propertyVal(params, "fromChrome", false);
assert.propertyVal(params, "forceForeground", false);
});
it("should call openTrustedLinkIn with the correct url, where, params and privacy args on OPEN_PRIVATE_WINDOW", () => {
const openTrustedLinkIn = sinon.stub();
@ -299,7 +299,7 @@ describe("PlacesFeed", () => {
assert.equal(url, "https://foo.com");
assert.equal(where, "window");
assert.propertyVal(params, "private", true);
assert.propertyVal(params, "fromChrome", false);
assert.propertyVal(params, "forceForeground", false);
});
it("should call openTrustedLinkIn with the correct url, where and params on OPEN_LINK", () => {
const openTrustedLinkIn = sinon.stub();
@ -320,7 +320,7 @@ describe("PlacesFeed", () => {
assert.equal(url, "https://foo.com");
assert.equal(where, "current");
assert.propertyVal(params, "private", false);
assert.propertyVal(params, "fromChrome", false);
assert.propertyVal(params, "forceForeground", false);
});
it("should open link with referrer on OPEN_LINK", () => {
const openTrustedLinkIn = sinon.stub();

View File

@ -1172,7 +1172,7 @@ export var PlacesUIUtils = {
/**
* Loads the node's URL in the appropriate tab or window.
* see also openUILinkIn
* see also URILoadingHelper's openWebLinkIn
*
* @param {object} aNode
* An uri result node.
@ -1840,21 +1840,17 @@ export var PlacesUIUtils = {
);
break;
case "placesCmd_open:privatewindow":
window.openUILinkIn(this.triggerNode.link, "window", {
triggeringPrincipal: Services.scriptSecurityManager.getSystemPrincipal(),
window.openTrustedLinkIn(this.triggerNode.link, "window", {
private: true,
});
break;
case "placesCmd_open:window":
window.openUILinkIn(this.triggerNode.link, "window", {
triggeringPrincipal: Services.scriptSecurityManager.getSystemPrincipal(),
window.openTrustedLinkIn(this.triggerNode.link, "window", {
private: false,
});
break;
case "placesCmd_open:tab": {
window.openUILinkIn(this.triggerNode.link, "tab", {
triggeringPrincipal: Services.scriptSecurityManager.getSystemPrincipal(),
});
window.openTrustedLinkIn(this.triggerNode.link, "tab");
}
}
},

View File

@ -139,7 +139,7 @@ class TextRecognitionModal {
this.linkEl.addEventListener("click", event => {
event.preventDefault();
this.openLinkIn(this.linkEl.href, "tab", {
fromChrome: true,
forceForeground: true,
triggeringPrincipal: Services.scriptSecurityManager.getSystemPrincipal(),
});
});

View File

@ -420,9 +420,7 @@ class _QuickSuggest {
switch (params.choice) {
case ONBOARDING_CHOICE.LEARN_MORE_1:
case ONBOARDING_CHOICE.LEARN_MORE_2:
win.openTrustedLinkIn(this.HELP_URL, "tab", {
fromChrome: true,
});
win.openTrustedLinkIn(this.HELP_URL, "tab");
break;
case ONBOARDING_CHOICE.ACCEPT_2:
case ONBOARDING_CHOICE.REJECT_2:

View File

@ -29,7 +29,7 @@ export const URILoadingHelper = {
return;
}
var aFromChrome = params.fromChrome;
var aForceForeground = params.forceForeground;
var aAllowThirdPartyFixup = params.allowThirdPartyFixup;
var aPostData = params.postData;
var aCharset = params.charset;
@ -326,7 +326,7 @@ export const URILoadingHelper = {
// `where` is "tab" or "tabshifted", so we'll load the link in a new tab.
loadInBackground = aInBackground;
if (loadInBackground == null) {
loadInBackground = aFromChrome
loadInBackground = aForceForeground
? false
: Services.prefs.getBoolPref("browser.tabs.loadInBackground");
}
@ -506,7 +506,7 @@ export const URILoadingHelper = {
* @param {Event | Object} event Event or JSON object representing an Event
* @param {Boolean | Object} aIgnoreButton
* Boolean or object with the same properties as
* accepted by openUILinkIn, plus "ignoreButton"
* accepted by openLinkIn, plus "ignoreButton"
* and "ignoreAlt".
* @param {Boolean} aIgnoreAlt
* @param {Boolean} aAllowThirdPartyFixup
@ -529,7 +529,7 @@ export const URILoadingHelper = {
if (aIgnoreButton && typeof aIgnoreButton == "object") {
params = aIgnoreButton;
// don't forward "ignoreButton" and "ignoreAlt" to openUILinkIn
// don't forward "ignoreButton" and "ignoreAlt" to openLinkIn
aIgnoreButton = params.ignoreButton;
aIgnoreAlt = params.ignoreAlt;
delete params.ignoreButton;
@ -550,66 +550,43 @@ export const URILoadingHelper = {
}
let where = BrowserUtils.whereToOpenLink(event, aIgnoreButton, aIgnoreAlt);
this.openUILinkIn(window, url, where, params);
params.forceForeground ??= true;
this.openLinkIn(window, url, where, params);
},
/* openUILinkIn opens a URL in a place specified by the parameter |where|.
/* openTrustedLinkIn will attempt to open the given URI using the SystemPrincipal
* as the trigeringPrincipal, unless a more specific Principal is provided.
*
* |where| can be:
* "current" current tab (if there aren't any browser windows, then in a new window instead)
* "tab" new tab (if there aren't any browser windows, then in a new window instead)
* "tabshifted" same as "tab" but in background if default is to select new tabs, and vice versa
* "window" new window
* "save" save to disk (with no filename hint!)
*
* DEPRECATION WARNING:
* USE -> openTrustedLinkIn(url, where, aParams) if the source is always
* a user event on a user- or product-specified URL (as
* opposed to URLs provided by a webpage)
* USE -> openWebLinkIn(url, where, aParams) if the URI should be loaded
* with a specific triggeringPrincipal, for instance, if
* the url was supplied by web content.
* DEPRECATED -> openUILinkIn(url, where, AllowThirdPartyFixup, aPostData, ...)
*
*
* allowThirdPartyFixup controls whether third party services such as Google's
* I Feel Lucky are allowed to interpret this URL. This parameter may be
* undefined, which is treated as false.
*
* Instead of aAllowThirdPartyFixup, you may also pass an object with any of
* these properties:
* allowThirdPartyFixup (boolean)
* fromChrome (boolean)
* postData (nsIInputStream)
* referrerInfo (nsIReferrerInfo)
* relatedToCurrent (boolean)
* skipTabAnimation (boolean)
* allowPinnedTabHostChange (boolean)
* allowPopups (boolean)
* userContextId (unsigned int)
* targetBrowser (XUL browser)
* Otherwise, parameters are the same as openLinkIn, but we will set `forceForeground`
* to true.
*/
openUILinkIn(
window,
url,
where,
aAllowThirdPartyFixup,
aPostData,
aReferrerInfo
) {
var params;
if (typeof aAllowThirdPartyFixup == "object") {
params = aAllowThirdPartyFixup;
openTrustedLinkIn(window, url, where, params = {}) {
if (!params.triggeringPrincipal) {
params.triggeringPrincipal = Services.scriptSecurityManager.getSystemPrincipal();
}
if (!params || !params.triggeringPrincipal) {
throw new Error(
"Required argument triggeringPrincipal missing within openUILinkIn"
params.forceForeground ??= true;
this.openLinkIn(window, url, where, params);
},
/* openWebLinkIn will attempt to open the given URI using the NullPrincipal
* as the triggeringPrincipal, unless a more specific Principal is provided.
*
* Otherwise, parameters are the same as openLinkIn, but we will set `forceForeground`
* to true.
*/
openWebLinkIn(window, url, where, params = {}) {
if (!params.triggeringPrincipal) {
params.triggeringPrincipal = Services.scriptSecurityManager.createNullPrincipal(
{}
);
}
params.fromChrome = params.fromChrome ?? true;
if (params.triggeringPrincipal.isSystemPrincipal) {
throw new Error(
"System principal should never be passed into openWebLinkIn()"
);
}
params.forceForeground ??= true;
this.openLinkIn(window, url, where, params);
},
};

View File

@ -91,10 +91,7 @@ export var ResetProfile = {
if (params.learnMore) {
win.openTrustedLinkIn(
"https://support.mozilla.org/kb/refresh-firefox-reset-add-ons-and-settings",
"tab",
{
fromChrome: true,
}
"tab"
);
return;
}

View File

@ -698,12 +698,7 @@ class SearchAddons extends HTMLElement {
let browser = getBrowserElement();
let chromewin = browser.ownerGlobal;
chromewin.openLinkIn(url, "tab", {
fromChrome: true,
triggeringPrincipal: Services.scriptSecurityManager.createNullPrincipal(
{}
),
});
chromewin.openWebLinkIn(url, "tab");
AMTelemetry.recordLinkEvent({
object: "aboutAddons",
@ -2714,13 +2709,7 @@ class AddonCard extends HTMLElement {
break;
case "contribute":
this.recordActionEvent("contribute");
// prettier-ignore
windowRoot.ownerGlobal.openUILinkIn(addon.contributionURL, "tab", {
triggeringPrincipal:
Services.scriptSecurityManager.createNullPrincipal(
{}
),
});
windowRoot.ownerGlobal.openWebLinkIn(addon.contributionURL, "tab");
break;
case "preferences":
if (getOptionsType(addon) == "tab") {