commit a6f907d2143c15b3e89fd18ddc2a7f42e16e0c02
parent 63c227351d72ec643454629f23ab7dd1b2daaef0
Author: Sandor Molnar <smolnar@mozilla.com>
Date: Wed, 7 Jan 2026 16:43:39 +0200
Revert "Bug 2008859 - Add notifyShown callback to track LNA prompt display state. r=emz,necko-reviewers,jesup" for causing bc failures @ browser_PermissionUI_prompts.js
This reverts commit ae635f4b7a9a57b14bd7d6c00f80f27534359426.
Diffstat:
8 files changed, 38 insertions(+), 225 deletions(-)
diff --git a/browser/modules/PermissionUI.sys.mjs b/browser/modules/PermissionUI.sys.mjs
@@ -1034,13 +1034,6 @@ class LNAPermissionPromptBase extends PermissionPromptForRequest {
this.request = request;
}
- onBeforeShow() {
- // Notify LNAPermissionRequest that the prompt is being shown.
- // This triggers telemetry recording and notifies nsHttpChannel.
- this.request.notifyShown();
- return true;
- }
-
onShown() {
this.#startTimeoutTimer();
}
diff --git a/dom/base/nsContentPermissionHelper.cpp b/dom/base/nsContentPermissionHelper.cpp
@@ -471,13 +471,6 @@ ContentPermissionRequestBase::GetElement(Element** aElement) {
}
NS_IMETHODIMP
-ContentPermissionRequestBase::NotifyShown() {
- // Default implementation does nothing.
- // Subclasses can override to perform actions when prompt is shown.
- return NS_OK;
-}
-
-NS_IMETHODIMP
ContentPermissionRequestBase::GetHasValidTransientUserGestureActivation(
bool* aHasValidTransientUserGestureActivation) {
*aHasValidTransientUserGestureActivation =
@@ -766,14 +759,6 @@ nsContentPermissionRequestProxy::GetIsRequestDelegatedToUnsafeThirdParty(
}
NS_IMETHODIMP
-nsContentPermissionRequestProxy::NotifyShown() {
- // This is a proxy class that forwards to content process.
- // The actual NotifyShown() logic is handled by the real implementation
- // in the content process, so we don't need to do anything here.
- return NS_OK;
-}
-
-NS_IMETHODIMP
nsContentPermissionRequestProxy::Cancel() {
if (mParent == nullptr) {
return NS_ERROR_FAILURE;
diff --git a/dom/base/nsContentPermissionHelper.h b/dom/base/nsContentPermissionHelper.h
@@ -105,7 +105,6 @@ class ContentPermissionRequestBase : public nsIContentPermissionRequest {
bool* aHasValidTransientUserGestureActivation) override;
NS_IMETHOD GetIsRequestDelegatedToUnsafeThirdParty(
bool* aIsRequestDelegatedToUnsafeThirdParty) override;
- NS_IMETHOD NotifyShown(void) override;
// Overrides for Allow() and Cancel() aren't provided by this class.
// That is the responsibility of the subclasses.
diff --git a/dom/interfaces/base/nsIContentPermissionPrompt.idl b/dom/interfaces/base/nsIContentPermissionPrompt.idl
@@ -76,13 +76,6 @@ interface nsIContentPermissionRequest : nsISupports {
nsIPrincipal getDelegatePrincipal(in ACString aType);
/**
- * Notify that the permission prompt has been shown to the user.
- * This is called when the prompt UI is displayed, before the user
- * makes a decision to allow or deny.
- */
- void notifyShown();
-
- /**
* allow or cancel the request
*/
[can_run_script]
diff --git a/netwerk/base/LNAPermissionRequest.cpp b/netwerk/base/LNAPermissionRequest.cpp
@@ -89,7 +89,7 @@ LNAPermissionRequest::GetElement(mozilla::dom::Element** aElement) {
NS_IMETHODIMP
LNAPermissionRequest::Cancel() {
// callback to the http channel on the prompt failure result
- mPermissionPromptCallback(false, mType, mPromptWasShown);
+ mPermissionPromptCallback(false, mType);
return NS_OK;
}
@@ -97,39 +97,7 @@ LNAPermissionRequest::Cancel() {
NS_IMETHODIMP
LNAPermissionRequest::Allow(JS::Handle<JS::Value> aChoices) {
// callback to the http channel on the prompt success result
- mPermissionPromptCallback(true, mType, mPromptWasShown);
- return NS_OK;
-}
-
-// callback when the permission prompt is shown
-NS_IMETHODIMP
-LNAPermissionRequest::NotifyShown() {
- // Mark that the prompt was shown to the user
- mPromptWasShown = true;
-
- // Record telemetry for permission prompts shown to users
- if (mType.Equals(LOCAL_HOST_PERMISSION_KEY)) {
- if (mIsRequestDelegatedToUnsafeThirdParty) {
- mozilla::glean::networking::local_network_access_prompts_shown
- .Get("localhost_cross_site"_ns)
- .Add(1);
- } else {
- mozilla::glean::networking::local_network_access_prompts_shown
- .Get("localhost"_ns)
- .Add(1);
- }
- } else if (mType.Equals(LOCAL_NETWORK_PERMISSION_KEY)) {
- if (mIsRequestDelegatedToUnsafeThirdParty) {
- mozilla::glean::networking::local_network_access_prompts_shown
- .Get("local_network_cross_site"_ns)
- .Add(1);
- } else {
- mozilla::glean::networking::local_network_access_prompts_shown
- .Get("local_network"_ns)
- .Add(1);
- }
- }
-
+ mPermissionPromptCallback(true, mType);
return NS_OK;
}
@@ -162,6 +130,29 @@ nsresult LNAPermissionRequest::RequestPermission() {
return Cancel();
}
+ // Record telemetry for permission prompts shown to users
+ if (mType.Equals(LOCAL_HOST_PERMISSION_KEY)) {
+ if (mIsRequestDelegatedToUnsafeThirdParty) {
+ mozilla::glean::networking::local_network_access_prompts_shown
+ .Get("localhost_cross_site"_ns)
+ .Add(1);
+ } else {
+ mozilla::glean::networking::local_network_access_prompts_shown
+ .Get("localhost"_ns)
+ .Add(1);
+ }
+ } else if (mType.Equals(LOCAL_NETWORK_PERMISSION_KEY)) {
+ if (mIsRequestDelegatedToUnsafeThirdParty) {
+ mozilla::glean::networking::local_network_access_prompts_shown
+ .Get("local_network_cross_site"_ns)
+ .Add(1);
+ } else {
+ mozilla::glean::networking::local_network_access_prompts_shown
+ .Get("local_network"_ns)
+ .Add(1);
+ }
+ }
+
if (NS_SUCCEEDED(
dom::nsContentPermissionUtils::AskPermission(this, mWindow))) {
// Here we could be getting synchronous callback from the prompts depending
diff --git a/netwerk/base/LNAPermissionRequest.h b/netwerk/base/LNAPermissionRequest.h
@@ -15,8 +15,7 @@ static constexpr nsLiteralCString LOCAL_HOST_PERMISSION_KEY = "localhost"_ns;
static constexpr nsLiteralCString LOCAL_NETWORK_PERMISSION_KEY =
"local-network"_ns;
-using PermissionPromptCallback =
- std::function<void(bool granted, const nsACString& type, bool promptShown)>;
+using PermissionPromptCallback = std::function<void(bool, const nsACString&)>;
/**
* Handles permission dialog management for local network accesses
@@ -34,7 +33,6 @@ class LNAPermissionRequest final : public dom::ContentPermissionRequestBase {
NS_IMETHOD
Cancel(void) override;
NS_IMETHOD Allow(JS::Handle<JS::Value> choices) override;
- NS_IMETHOD NotifyShown(void) override;
NS_IMETHOD GetElement(mozilla::dom::Element** aElement) override;
nsresult RequestPermission();
@@ -43,7 +41,6 @@ class LNAPermissionRequest final : public dom::ContentPermissionRequestBase {
~LNAPermissionRequest() = default;
nsCOMPtr<nsILoadInfo> mLoadInfo;
PermissionPromptCallback mPermissionPromptCallback;
- bool mPromptWasShown = false;
};
} // namespace mozilla::net
diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -8829,36 +8829,20 @@ nsresult nsHttpChannel::ProcessLNAActions() {
UpdateLocalNetworkAccessPermissions(permissionKey);
if (LNAPermission::Granted == permissionUpdateResult) {
- // permission granted (auto-allow via permanent permission)
- mLNAPromptAction.AssignLiteral("auto_allow");
+ // permission granted
return OnPermissionPromptResult(true, permissionKey);
}
if (LNAPermission::Denied == permissionUpdateResult) {
- // permission denied (auto-deny via permanent permission)
- mLNAPromptAction.AssignLiteral("auto_deny");
+ // permission denied
return OnPermissionPromptResult(false, permissionKey);
}
// If we get here, we don't have any permission to access the local
// host/network. We need to prompt the user for action
- auto permissionPromptCallback =
- [self = RefPtr{this}](bool aPermissionGranted, const nsACString& aType,
- bool aPromptShown) -> void {
- // Set mLNAPromptAction based on whether prompt was shown and user response
- if (aPromptShown) {
- if (aPermissionGranted) {
- self->mLNAPromptAction.AssignLiteral("prompt_allow");
- } else {
- self->mLNAPromptAction.AssignLiteral("prompt_deny");
- }
- } else {
- if (aPermissionGranted) {
- self->mLNAPromptAction.AssignLiteral("auto_allow");
- } else {
- self->mLNAPromptAction.AssignLiteral("auto_deny");
- }
- }
+ auto permissionPromptCallback = [self = RefPtr{this}](
+ bool aPermissionGranted,
+ const nsACString& aType) -> void {
self->OnPermissionPromptResult(aPermissionGranted, aType);
};
@@ -9229,6 +9213,13 @@ nsresult nsHttpChannel::OnPermissionPromptResult(bool aGranted,
const nsACString& aType) {
mWaitingForLNAPermission = false;
+ // Set prompt action for telemetry
+ if (aGranted) {
+ mLNAPromptAction.AssignLiteral("allow");
+ } else {
+ mLNAPromptAction.AssignLiteral("deny");
+ }
+
if (aGranted) {
LOG(
("nsHttpChannel::OnPermissionPromptResult [this=%p] "
diff --git a/netwerk/test/browser/browser_test_local_network_access.js b/netwerk/test/browser/browser_test_local_network_access.js
@@ -603,139 +603,3 @@ add_task(async function test_lna_prompt_timeout() {
await SpecialPowers.popPrefEnv();
});
-
-// Test that telemetry is recorded when LNA prompt is shown
-// and not incremented for subsequent requests with cached permission
-add_task(async function test_lna_prompt_telemetry() {
- await restorePermissions();
-
- // Reset telemetry
- Services.fog.testResetFOG();
- await SpecialPowers.pushPrefEnv({
- set: [["network.lna.address_space.public.override", "127.0.0.1:4443"]],
- });
-
- const rand1 = Math.random();
- const tab = await BrowserTestUtils.openNewForegroundTab(
- gBrowser,
- `${baseURL}page_with_non_trackers.html?test=fetch&rand=${rand1}`
- );
-
- // Wait for the prompt to appear
- await BrowserTestUtils.waitForEvent(PopupNotifications.panel, "popupshown");
-
- // Verify telemetry was recorded
- let metricValue =
- await Glean.networking.localNetworkAccessPromptsShown.localhost.testGetValue();
- is(metricValue, 1, "Should record telemetry when localhost prompt is shown");
-
- // Grant permission
- clickDoorhangerButton(
- PROMPT_ALLOW_BUTTON,
- gBrowser.selectedBrowser,
- "localhost"
- );
-
- // Wait for permission to be saved
- // eslint-disable-next-line mozilla/no-arbitrary-setTimeout
- await new Promise(resolve => setTimeout(resolve, 300));
-
- // Make a second request in the same tab with cached permission
- const rand2 = Math.random();
- const promise = observeAndCheck(
- "fetch",
- rand2,
- Cr.NS_OK,
- "Second request should succeed without prompt"
- );
- await SpecialPowers.spawn(tab.linkedBrowser, [rand2], async rand => {
- await content.fetch(`http://localhost:21555/?type=fetch&rand=${rand}`);
- });
- await promise;
-
- // Verify telemetry was not incremented
- metricValue =
- await Glean.networking.localNetworkAccessPromptsShown.localhost.testGetValue();
- is(
- metricValue,
- 1,
- "Telemetry should not increment for requests with cached permission"
- );
-
- gBrowser.removeTab(tab);
- await SpecialPowers.popPrefEnv();
-});
-
-// Test that telemetry is recorded when user denies LNA prompt
-// and not incremented for subsequent requests with temporary deny permission
-add_task(async function test_lna_prompt_telemetry_deny() {
- await restorePermissions();
-
- // Reset telemetry
- Services.fog.testResetFOG();
- await SpecialPowers.pushPrefEnv({
- set: [["network.lna.address_space.public.override", "127.0.0.1:4443"]],
- });
-
- const rand1 = Math.random();
- const promise1 = observeAndCheck(
- "fetch",
- rand1,
- Cr.NS_ERROR_LOCAL_NETWORK_ACCESS_DENIED,
- "First request should be denied"
- );
- const tab = await BrowserTestUtils.openNewForegroundTab(
- gBrowser,
- `${baseURL}page_with_non_trackers.html?test=fetch&rand=${rand1}`
- );
-
- // Wait for the prompt to appear
- await BrowserTestUtils.waitForEvent(PopupNotifications.panel, "popupshown");
-
- // Verify telemetry was recorded
- let metricValue =
- await Glean.networking.localNetworkAccessPromptsShown.localhost.testGetValue();
- is(metricValue, 1, "Should record telemetry when localhost prompt is shown");
-
- // Deny permission
- clickDoorhangerButton(
- PROMPT_NOT_NOW_BUTTON,
- gBrowser.selectedBrowser,
- "localhost"
- );
-
- await promise1;
-
- // Wait for permission to be saved
- // eslint-disable-next-line mozilla/no-arbitrary-setTimeout
- await new Promise(resolve => setTimeout(resolve, 300));
-
- // Make a second request - should be auto-denied without showing prompt
- // because a temporary deny permission was saved
- const rand2 = Math.random();
- const promise2 = observeAndCheck(
- "fetch",
- rand2,
- Cr.NS_ERROR_LOCAL_NETWORK_ACCESS_DENIED,
- "Second request should be auto-denied with temporary permission"
- );
- await SpecialPowers.spawn(tab.linkedBrowser, [rand2], async rand => {
- await content
- .fetch(`http://localhost:21555/?type=fetch&rand=${rand}`)
- .catch(() => {});
- });
-
- await promise2;
-
- // Verify telemetry was not incremented (no prompt shown with temporary deny)
- metricValue =
- await Glean.networking.localNetworkAccessPromptsShown.localhost.testGetValue();
- is(
- metricValue,
- 1,
- "Telemetry should not increment for requests with temporary deny permission"
- );
-
- gBrowser.removeTab(tab);
- await SpecialPowers.popPrefEnv();
-});
