From 859421e601fdbc9fe334ed0d73093b3e045111da Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Mon, 29 Jul 2019 18:47:37 -0700 Subject: [PATCH 01/12] fix: define behavior for out-of-bounds setOpacity --- shell/browser/native_window_views.cc | 11 +++++++++-- spec-main/api-browser-window-spec.ts | 7 +++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/shell/browser/native_window_views.cc b/shell/browser/native_window_views.cc index c7387bb353ad7..3e981072b23eb 100644 --- a/shell/browser/native_window_views.cc +++ b/shell/browser/native_window_views.cc @@ -880,6 +880,13 @@ bool NativeWindowViews::HasShadow() { void NativeWindowViews::SetOpacity(const double opacity) { #if defined(OS_WIN) + double boundedOpacity = opacity; + if (opacity > 1) { + boundedOpacity = 1; + } else if (opacity < 0) { + boundedOpacity = 0; + } + HWND hwnd = GetAcceleratedWidget(); if (!layered_) { LONG ex_style = ::GetWindowLong(hwnd, GWL_EXSTYLE); @@ -887,9 +894,9 @@ 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); #endif - opacity_ = opacity; + opacity_ = boundedOpacity; } double NativeWindowViews::GetOpacity() { diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts index d985a85b50a57..0a8fd69968272 100644 --- a/spec-main/api-browser-window-spec.ts +++ b/spec-main/api-browser-window-spec.ts @@ -1257,6 +1257,13 @@ describe('BrowserWindow module', () => { expect(w.getOpacity()).to.equal(1.0) }).to.not.throw() }) + it.only('does not change opacity if given number out of bounds', () => { + const w = new BrowserWindow({ show: false, opacity: 0.5 }) + w.setOpacity(100) + expect(w.getOpacity()).to.equal(1) + w.setOpacity(-100) + expect(w.getOpacity()).to.equal(0) + }) }) describe('BrowserWindow.setShape(rects)', () => { From 68bdd16a945fd2b99aedbb175d67f3066263f70e Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Mon, 29 Jul 2019 19:13:22 -0700 Subject: [PATCH 02/12] fix linux issue --- shell/browser/native_window_views.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/shell/browser/native_window_views.cc b/shell/browser/native_window_views.cc index 3e981072b23eb..b6af79e6fc200 100644 --- a/shell/browser/native_window_views.cc +++ b/shell/browser/native_window_views.cc @@ -895,8 +895,9 @@ void NativeWindowViews::SetOpacity(const double opacity) { layered_ = true; } ::SetLayeredWindowAttributes(hwnd, 0, boundedOpacity * 255, LWA_ALPHA); +#else + opacity_ = opacity; #endif - opacity_ = boundedOpacity; } double NativeWindowViews::GetOpacity() { From e2a597b925f85649cd23e485de00244ec7e91b1a Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Mon, 29 Jul 2019 20:14:11 -0700 Subject: [PATCH 03/12] fix getOpacity behaviour --- shell/browser/native_window_mac.mm | 9 ++++++++- shell/browser/native_window_views.cc | 5 ++--- spec-main/api-browser-window-spec.ts | 7 ++++--- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/shell/browser/native_window_mac.mm b/shell/browser/native_window_mac.mm index 9afe85b2a5f1a..a650d8a52bcb3 100644 --- a/shell/browser/native_window_mac.mm +++ b/shell/browser/native_window_mac.mm @@ -1032,7 +1032,14 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) { } void NativeWindowMac::SetOpacity(const double opacity) { - [window_ setAlphaValue:opacity]; + double boundedOpacity = opacity; + if (opacity < 0) { + boundedOpacity = 0; + } else if (opacity > 1) { + boundedOpacity = 1; + } + + [window_ setAlphaValue:boundedOpacity]; } double NativeWindowMac::GetOpacity() { diff --git a/shell/browser/native_window_views.cc b/shell/browser/native_window_views.cc index b6af79e6fc200..7a3774be3d721 100644 --- a/shell/browser/native_window_views.cc +++ b/shell/browser/native_window_views.cc @@ -879,7 +879,6 @@ bool NativeWindowViews::HasShadow() { } void NativeWindowViews::SetOpacity(const double opacity) { -#if defined(OS_WIN) double boundedOpacity = opacity; if (opacity > 1) { boundedOpacity = 1; @@ -887,6 +886,7 @@ void NativeWindowViews::SetOpacity(const double opacity) { boundedOpacity = 0; } +#if defined(OS_WIN) HWND hwnd = GetAcceleratedWidget(); if (!layered_) { LONG ex_style = ::GetWindowLong(hwnd, GWL_EXSTYLE); @@ -895,9 +895,8 @@ void NativeWindowViews::SetOpacity(const double opacity) { layered_ = true; } ::SetLayeredWindowAttributes(hwnd, 0, boundedOpacity * 255, LWA_ALPHA); -#else - opacity_ = opacity; #endif + opacity_ = opacity; } double NativeWindowViews::GetOpacity() { diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts index 0a8fd69968272..740c1eef0e987 100644 --- a/spec-main/api-browser-window-spec.ts +++ b/spec-main/api-browser-window-spec.ts @@ -1257,12 +1257,13 @@ describe('BrowserWindow module', () => { expect(w.getOpacity()).to.equal(1.0) }).to.not.throw() }) - it.only('does not change opacity if given number out of bounds', () => { + + it('does not change opacity if given number out of bounds', () => { const w = new BrowserWindow({ show: false, opacity: 0.5 }) w.setOpacity(100) - expect(w.getOpacity()).to.equal(1) + expect(w.getOpacity()).to.equal(1.0) w.setOpacity(-100) - expect(w.getOpacity()).to.equal(0) + expect(w.getOpacity()).to.equal(0.0) }) }) From 48984cf0cf1d2d086a50c272d848936fec12bc0f Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Mon, 29 Jul 2019 20:21:31 -0700 Subject: [PATCH 04/12] wrong variable --- shell/browser/native_window_views.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/browser/native_window_views.cc b/shell/browser/native_window_views.cc index 7a3774be3d721..d24b70a4bad7c 100644 --- a/shell/browser/native_window_views.cc +++ b/shell/browser/native_window_views.cc @@ -896,7 +896,7 @@ void NativeWindowViews::SetOpacity(const double opacity) { } ::SetLayeredWindowAttributes(hwnd, 0, boundedOpacity * 255, LWA_ALPHA); #endif - opacity_ = opacity; + opacity_ = boundedOpacity; } double NativeWindowViews::GetOpacity() { From bfb49cecaa56d81bd9d6ad7017bf861147819f9e Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Tue, 30 Jul 2019 09:11:51 -0700 Subject: [PATCH 05/12] normalize more stuff --- shell/browser/native_window_views.cc | 6 ++++-- spec-main/api-browser-window-spec.ts | 6 ++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/shell/browser/native_window_views.cc b/shell/browser/native_window_views.cc index d24b70a4bad7c..010dddabe3495 100644 --- a/shell/browser/native_window_views.cc +++ b/shell/browser/native_window_views.cc @@ -879,6 +879,7 @@ bool NativeWindowViews::HasShadow() { } void NativeWindowViews::SetOpacity(const double opacity) { +#if defined(OS_WIN) double boundedOpacity = opacity; if (opacity > 1) { boundedOpacity = 1; @@ -886,7 +887,6 @@ void NativeWindowViews::SetOpacity(const double opacity) { boundedOpacity = 0; } -#if defined(OS_WIN) HWND hwnd = GetAcceleratedWidget(); if (!layered_) { LONG ex_style = ::GetWindowLong(hwnd, GWL_EXSTYLE); @@ -895,8 +895,10 @@ void NativeWindowViews::SetOpacity(const double opacity) { layered_ = true; } ::SetLayeredWindowAttributes(hwnd, 0, boundedOpacity * 255, LWA_ALPHA); -#endif opacity_ = boundedOpacity; +#else + opacity_ = 1; // setOpacity unsupported on Linux +#endif } double NativeWindowViews::GetOpacity() { diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts index 740c1eef0e987..c08ac5dc70198 100644 --- a/spec-main/api-browser-window-spec.ts +++ b/spec-main/api-browser-window-spec.ts @@ -1241,6 +1241,12 @@ describe('BrowserWindow module', () => { }) describe('BrowserWindow.setOpacity(opacity)', () => { + before(function () { + // setOpacity() unsupported on Linux + if (process.platform === 'linux') { + this.skip() + } + }) afterEach(closeAllWindows) it('make window with initial opacity', () => { const w = new BrowserWindow({ show: false, opacity: 0.5 }) From 7d34c20b36c89e809eff4a3dda867214e7087e48 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Tue, 30 Jul 2019 09:26:00 -0700 Subject: [PATCH 06/12] docs --- docs/api/browser-window.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/api/browser-window.md b/docs/api/browser-window.md index 3fcac5ab91953..6789422958226 100644 --- a/docs/api/browser-window.md +++ b/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_ -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_ From 83fc694b890d7270a1b1a754846e8766b3f89434 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Tue, 30 Jul 2019 14:36:11 -0700 Subject: [PATCH 07/12] test: use ifdescribe helper --- spec-main/api-browser-window-spec.ts | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts index c08ac5dc70198..5a57b0382f0ea 100644 --- a/spec-main/api-browser-window-spec.ts +++ b/spec-main/api-browser-window-spec.ts @@ -7,13 +7,11 @@ 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 -const ifit = (condition: boolean) => (condition ? it : it.skip) -const ifdescribe = (condition: boolean) => (condition ? describe : describe.skip) - chai.use(chaiAsPromised) const fixtures = path.resolve(__dirname, '..', 'spec', 'fixtures') @@ -1240,13 +1238,8 @@ describe('BrowserWindow module', () => { }) }) - describe('BrowserWindow.setOpacity(opacity)', () => { - before(function () { - // setOpacity() unsupported on Linux - if (process.platform === 'linux') { - this.skip() - } - }) + + ifdescribe(process.platform !== 'linux')('BrowserWindow.setOpacity(opacity)', () => { afterEach(closeAllWindows) it('make window with initial opacity', () => { const w = new BrowserWindow({ show: false, opacity: 0.5 }) From a9701609ffb70d701d76d6a4973f72cc9b001975 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 31 Jul 2019 16:17:53 -0700 Subject: [PATCH 08/12] Update spec-main/api-browser-window-spec.ts Co-Authored-By: Charles Kerr --- spec-main/api-browser-window-spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts index 5a57b0382f0ea..ce291b81bb00f 100644 --- a/spec-main/api-browser-window-spec.ts +++ b/spec-main/api-browser-window-spec.ts @@ -1257,7 +1257,7 @@ describe('BrowserWindow module', () => { }).to.not.throw() }) - it('does not change opacity if given number out of bounds', () => { + 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) From b993a5ee3500420da3fba26f210adf55bc4ec5d8 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 31 Jul 2019 16:29:33 -0700 Subject: [PATCH 09/12] fixes --- docs/api/browser-window.md | 2 +- shell/browser/native_window_mac.mm | 9 ++------- shell/browser/native_window_views.cc | 11 +++-------- spec-main/api-browser-window-spec.ts | 2 +- 4 files changed, 7 insertions(+), 17 deletions(-) diff --git a/docs/api/browser-window.md b/docs/api/browser-window.md index 6789422958226..83568ec599075 100644 --- a/docs/api/browser-window.md +++ b/docs/api/browser-window.md @@ -1458,7 +1458,7 @@ On Windows and Linux always returns 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). On Linux, always returns 1. diff --git a/shell/browser/native_window_mac.mm b/shell/browser/native_window_mac.mm index a650d8a52bcb3..bc245d948a436 100644 --- a/shell/browser/native_window_mac.mm +++ b/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,13 +1033,7 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) { } void NativeWindowMac::SetOpacity(const double opacity) { - double boundedOpacity = opacity; - if (opacity < 0) { - boundedOpacity = 0; - } else if (opacity > 1) { - boundedOpacity = 1; - } - + double boundedOpacity = base::ClampToRange(opacity, 0.0, 1.0); [window_ setAlphaValue:boundedOpacity]; } diff --git a/shell/browser/native_window_views.cc b/shell/browser/native_window_views.cc index 010dddabe3495..1852792529b46 100644 --- a/shell/browser/native_window_views.cc +++ b/shell/browser/native_window_views.cc @@ -13,6 +13,7 @@ #include #include +#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,13 +881,7 @@ bool NativeWindowViews::HasShadow() { void NativeWindowViews::SetOpacity(const double opacity) { #if defined(OS_WIN) - double boundedOpacity = opacity; - if (opacity > 1) { - boundedOpacity = 1; - } else if (opacity < 0) { - boundedOpacity = 0; - } - + double boundedOpacity = base::ClampToRange(opacity, 0.0, 1.0); HWND hwnd = GetAcceleratedWidget(); if (!layered_) { LONG ex_style = ::GetWindowLong(hwnd, GWL_EXSTYLE); @@ -897,7 +892,7 @@ void NativeWindowViews::SetOpacity(const double opacity) { ::SetLayeredWindowAttributes(hwnd, 0, boundedOpacity * 255, LWA_ALPHA); opacity_ = boundedOpacity; #else - opacity_ = 1; // setOpacity unsupported on Linux + opacity_ = 1.0; // setOpacity unsupported on Linux #endif } diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts index 5a57b0382f0ea..ce291b81bb00f 100644 --- a/spec-main/api-browser-window-spec.ts +++ b/spec-main/api-browser-window-spec.ts @@ -1257,7 +1257,7 @@ describe('BrowserWindow module', () => { }).to.not.throw() }) - it('does not change opacity if given number out of bounds', () => { + 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) From db9cefd25845aeff02c21cd0aa8b4bbe1bf3482c Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Thu, 1 Aug 2019 15:18:14 -0700 Subject: [PATCH 10/12] more tests!!! --- spec-main/api-browser-window-spec.ts | 51 +++++++++++++++++----------- 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts index ce291b81bb00f..eed92d9f66ed3 100644 --- a/spec-main/api-browser-window-spec.ts +++ b/spec-main/api-browser-window-spec.ts @@ -1239,30 +1239,43 @@ describe('BrowserWindow module', () => { }) - ifdescribe(process.platform !== 'linux')('BrowserWindow.setOpacity(opacity)', () => { + describe('BrowserWindow.setOpacity(opacity)', () => { afterEach(closeAllWindows) - it('make window with initial opacity', () => { - const w = new BrowserWindow({ show: false, opacity: 0.5 }) - expect(w.getOpacity()).to.equal(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) + + ifdescribe(process.platform !== 'linux')(('Windows and Mac'), () => { + it('make window with initial opacity', () => { + const w = new BrowserWindow({ show: false, opacity: 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) + }) }) - 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) - 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) + }) }) }) From 75b4f46b10245c1ed4b6227bbbfb8472407a6805 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Sat, 3 Aug 2019 20:10:54 -0700 Subject: [PATCH 11/12] Update shell/browser/native_window_views.cc Co-Authored-By: Charles Kerr --- shell/browser/native_window_views.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/browser/native_window_views.cc b/shell/browser/native_window_views.cc index 1852792529b46..cc030cf83c85b 100644 --- a/shell/browser/native_window_views.cc +++ b/shell/browser/native_window_views.cc @@ -881,7 +881,7 @@ bool NativeWindowViews::HasShadow() { void NativeWindowViews::SetOpacity(const double opacity) { #if defined(OS_WIN) - double boundedOpacity = base::ClampToRange(opacity, 0.0, 1.0); + const double boundedOpacity = base::ClampToRange(opacity, 0.0, 1.0); HWND hwnd = GetAcceleratedWidget(); if (!layered_) { LONG ex_style = ::GetWindowLong(hwnd, GWL_EXSTYLE); From 688906b4826ed95d6d7df81bbd83879c0c9b1045 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Sat, 3 Aug 2019 20:11:00 -0700 Subject: [PATCH 12/12] Update shell/browser/native_window_mac.mm Co-Authored-By: Charles Kerr --- shell/browser/native_window_mac.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/browser/native_window_mac.mm b/shell/browser/native_window_mac.mm index bc245d948a436..f70975f6c4fd7 100644 --- a/shell/browser/native_window_mac.mm +++ b/shell/browser/native_window_mac.mm @@ -1033,7 +1033,7 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) { } void NativeWindowMac::SetOpacity(const double opacity) { - double boundedOpacity = base::ClampToRange(opacity, 0.0, 1.0); + const double boundedOpacity = base::ClampToRange(opacity, 0.0, 1.0); [window_ setAlphaValue:boundedOpacity]; }