fix: normalize behavior of win.setOpacity() for invalid number values across operating systems (#19535)

* fix: define behavior for out-of-bounds setOpacity

* fix linux issue

* fix getOpacity behaviour

* wrong variable

* normalize more stuff

* docs

* test: use ifdescribe helper

* Update spec-main/api-browser-window-spec.ts

Co-Authored-By: Charles Kerr <ckerr@github.com>

* fixes

* more tests!!!

* Update shell/browser/native_window_views.cc

Co-Authored-By: Charles Kerr <ckerr@github.com>

* Update shell/browser/native_window_mac.mm

Co-Authored-By: Charles Kerr <ckerr@github.com>

docs/api/browser-window.md

@@ -1455,11 +1455,13 @@ On Windows and Linux always returns
 
 * `opacity` Number - between 0.0 (fully transparent) and 1.0 (fully opaque)
 
-Sets the opacity of the window. On Linux does nothing.
+Sets the opacity of the window. On Linux, does nothing. Out of bound number
+values are clamped to the [0, 1] range.
 
-#### `win.getOpacity()` _Windows_ _macOS_
+#### `win.getOpacity()`
 
-Returns `Number` - between 0.0 (fully transparent) and 1.0 (fully opaque)
+Returns `Number` - between 0.0 (fully transparent) and 1.0 (fully opaque). On
+Linux, always returns 1.
 
 #### `win.setShape(rects)` _Windows_ _Linux_ _Experimental_
 

shell/browser/native_window_mac.mm

@@ -12,6 +12,7 @@
 
 #include "base/mac/mac_util.h"
 #include "base/mac/scoped_cftyperef.h"
+#include "base/numerics/ranges.h"
 #include "base/strings/sys_string_conversions.h"
 #include "components/remote_cocoa/app_shim/native_widget_ns_window_bridge.h"
 #include "content/public/browser/browser_accessibility_state.h"
@@ -1032,7 +1033,8 @@ bool NativeWindowMac::HasShadow() {
 }
 
 void NativeWindowMac::SetOpacity(const double opacity) {
-  [window_ setAlphaValue:opacity];
+  const double boundedOpacity = base::ClampToRange(opacity, 0.0, 1.0);
+  [window_ setAlphaValue:boundedOpacity];
 }
 
 double NativeWindowMac::GetOpacity() {

shell/browser/native_window_views.cc

@@ -13,6 +13,7 @@
 #include <utility>
 #include <vector>
 
+#include "base/numerics/ranges.h"
 #include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "content/public/browser/browser_thread.h"
@@ -880,6 +881,7 @@ bool NativeWindowViews::HasShadow() {
 
 void NativeWindowViews::SetOpacity(const double opacity) {
 #if defined(OS_WIN)
+  const double boundedOpacity = base::ClampToRange(opacity, 0.0, 1.0);
   HWND hwnd = GetAcceleratedWidget();
   if (!layered_) {
     LONG ex_style = ::GetWindowLong(hwnd, GWL_EXSTYLE);
@@ -887,9 +889,11 @@ void NativeWindowViews::SetOpacity(const double opacity) {
     ::SetWindowLong(hwnd, GWL_EXSTYLE, ex_style);
     layered_ = true;
   }
-  ::SetLayeredWindowAttributes(hwnd, 0, opacity * 255, LWA_ALPHA);
+  ::SetLayeredWindowAttributes(hwnd, 0, boundedOpacity * 255, LWA_ALPHA);
+  opacity_ = boundedOpacity;
+#else
+  opacity_ = 1.0;  // setOpacity unsupported on Linux
 #endif
-  opacity_ = opacity;
 }
 
 double NativeWindowViews::GetOpacity() {

spec-main/api-browser-window-spec.ts

@@ -8,9 +8,10 @@ import * as qs from 'querystring'
 import * as http from 'http'
 import { AddressInfo } from 'net'
 import { app, BrowserWindow, BrowserView, ipcMain, OnBeforeSendHeadersListenerDetails, protocol, screen, webContents, session, WebContents } from 'electron'
-import { emittedOnce } from './events-helpers';
+
-import { closeWindow } from './window-helpers';
+import { emittedOnce } from './events-helpers'
 import { ifit, ifdescribe } from './spec-helpers'
+import { closeWindow } from './window-helpers'
 
 const { expect } = chai
 
@@ -1242,22 +1243,44 @@ describe('BrowserWindow module', () => {
     })
   })
 
+
   describe('BrowserWindow.setOpacity(opacity)', () => {
     afterEach(closeAllWindows)
-    it('make window with initial opacity', () => {
+
-      const w = new BrowserWindow({ show: false, opacity: 0.5 })
+    ifdescribe(process.platform !== 'linux')(('Windows and Mac'), () => {
-      expect(w.getOpacity()).to.equal(0.5)
+      it('make window with initial opacity', () => {
-    })
+        const w = new BrowserWindow({ show: false, opacity: 0.5 })
-    it('allows setting the opacity', () => {
-      const w = new BrowserWindow({ show: false })
-      expect(() => {
-        w.setOpacity(0.0)
-        expect(w.getOpacity()).to.equal(0.0)
-        w.setOpacity(0.5)
         expect(w.getOpacity()).to.equal(0.5)
-        w.setOpacity(1.0)
+      })
+      it('allows setting the opacity', () => {
+        const w = new BrowserWindow({ show: false })
+        expect(() => {
+          w.setOpacity(0.0)
+          expect(w.getOpacity()).to.equal(0.0)
+          w.setOpacity(0.5)
+          expect(w.getOpacity()).to.equal(0.5)
+          w.setOpacity(1.0)
+          expect(w.getOpacity()).to.equal(1.0)
+        }).to.not.throw()
+      })
+
+      it('clamps opacity to [0.0...1.0]', () => {
+        const w = new BrowserWindow({ show: false, opacity: 0.5 })
+        w.setOpacity(100)
         expect(w.getOpacity()).to.equal(1.0)
-      }).to.not.throw()
+        w.setOpacity(-100)
+        expect(w.getOpacity()).to.equal(0.0)
+      })
+    })
+
+    ifdescribe(process.platform === 'linux')(('Linux'), () => {
+      it('sets 1 regardless of parameter', () => {
+        const w = new BrowserWindow({ show: false })
+        w.setOpacity(0)
+        expect(w.getOpacity()).to.equal(1.0)
+        w.setOpacity(0.5)
+        expect(w.getOpacity()).to.equal(1.0)
+      })
     })
   })