From dbf887281767a7a597a6a9995c693964917e1b0b Mon Sep 17 00:00:00 2001
From: Erick Zhao <erick@hotmail.ca>
Date: Wed, 14 Aug 2019 18:10:06 -0700
Subject: [PATCH] fix: normalize behavior of `win.setOpacity()` for invalid
 number values across operating systems (#19723)

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

* expect -> assert

* assert 2

* fix `this` scoping

* fix equality

* tests
---
 atom/browser/native_window_mac.mm   |  4 ++-
 atom/browser/native_window_views.cc |  8 +++--
 spec/api-browser-window-spec.js     | 56 +++++++++++++++++++++--------
 3 files changed, 51 insertions(+), 17 deletions(-)

diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm
index 8ac4746a63fba..46570e307ddef 100644
--- a/atom/browser/native_window_mac.mm
+++ b/atom/browser/native_window_mac.mm
@@ -22,6 +22,7 @@
 #include "atom/common/options_switches.h"
 #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/bridged_native_widget_impl.h"
 #include "content/public/browser/browser_accessibility_state.h"
@@ -1047,7 +1048,8 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) {
 }
 
 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() {
diff --git a/atom/browser/native_window_views.cc b/atom/browser/native_window_views.cc
index dac82de74b14c..ccc4c3b51af5a 100644
--- a/atom/browser/native_window_views.cc
+++ b/atom/browser/native_window_views.cc
@@ -25,6 +25,7 @@
 #include "atom/common/draggable_region.h"
 #include "atom/common/native_mate_converters/image_converter.h"
 #include "atom/common/options_switches.h"
+#include "base/numerics/ranges.h"
 #include "base/strings/utf_string_conversions.h"
 #include "content/public/browser/browser_thread.h"
 #include "native_mate/dictionary.h"
@@ -866,6 +867,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);
@@ -873,9 +875,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() {
diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js
index e262afd09750a..2d6ca85e4f4ce 100644
--- a/spec/api-browser-window-spec.js
+++ b/spec/api-browser-window-spec.js
@@ -1214,23 +1214,51 @@ describe('BrowserWindow module', () => {
   })
 
   describe('BrowserWindow.setOpacity(opacity)', () => {
-    it('make window with initial opacity', () => {
-      w.destroy()
-      w = new BrowserWindow({
-        show: false,
-        width: 400,
-        height: 400,
-        opacity: 0.5
+    describe('Windows and Mac', () => {
+      before(function () {
+        if (process.platform === 'linux') {
+          this.skip()
+        }
       })
-      assert.strictEqual(w.getOpacity(), 0.5)
-    })
-    it('allows setting the opacity', () => {
-      assert.doesNotThrow(() => {
-        w.setOpacity(0.0)
+
+      it('makes a window with initial opacity', () => {
+        w.destroy()
+        w = new BrowserWindow({ show: false, opacity: 0.5 })
+        assert.strictEqual(w.getOpacity(), 0.5)
+      })
+
+      it('allows setting the opacity', () => {
+        assert.doesNotThrow(() => {
+          w.setOpacity(0.0)
+          assert.strictEqual(w.getOpacity(), 0.0)
+          w.setOpacity(0.5)
+          assert.strictEqual(w.getOpacity(), 0.5)
+          w.setOpacity(1.0)
+          assert.strictEqual(w.getOpacity(), 1.0)
+        })
+      })
+
+      it('clamps opacity to [0.0...1.0]', () => {
+        w.setOpacity(100)
+        assert.strictEqual(w.getOpacity(), 1.0)
+        w.setOpacity(-100)
         assert.strictEqual(w.getOpacity(), 0.0)
+      })
+    })
+
+    describe('Linux', () => {
+      before(function () {
+        if (process.platform !== 'linux') {
+          this.skip()
+        }
+      })
+
+      it('sets 1 regardless of parameter', () => {
+        w.destroy()
+        w = new BrowserWindow({ show: false, opacity: 0.5 })
+        w.setOpacity(0)
+        assert.strictEqual(w.getOpacity(), 1.0)
         w.setOpacity(0.5)
-        assert.strictEqual(w.getOpacity(), 0.5)
-        w.setOpacity(1.0)
         assert.strictEqual(w.getOpacity(), 1.0)
       })
     })