Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: update app module property support #22747

Merged
merged 1 commit into from Mar 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 0 additions & 12 deletions docs/api/app.md
Expand Up @@ -659,8 +659,6 @@ to the npm modules spec. You should usually also specify a `productName`
field, which is your application's full capitalized name, and which will be
preferred over `name` by Electron.

**[Deprecated](modernization/property-updates.md)**

### `app.setName(name)`

* `name` String
Expand All @@ -669,8 +667,6 @@ Overrides the current application's name.

**Note:** This function overrides the name used internally by Electron; it does not affect the name that the OS uses.

**[Deprecated](modernization/property-updates.md)**

### `app.getLocale()`

Returns `String` - The current application locale. Possible return values are documented [here](locales.md).
Expand Down Expand Up @@ -1089,14 +1085,10 @@ On macOS, it shows on the dock icon. On Linux, it only works for Unity launcher.
**Note:** Unity launcher requires the existence of a `.desktop` file to work,
for more information please read [Desktop Environment Integration][unity-requirement].

**[Deprecated](modernization/property-updates.md)**

### `app.getBadgeCount()` _Linux_ _macOS_

Returns `Integer` - The current value displayed in the counter badge.

**[Deprecated](modernization/property-updates.md)**

### `app.isUnityRunning()` _Linux_

Returns `Boolean` - Whether the current desktop environment is Unity launcher.
Expand Down Expand Up @@ -1171,8 +1163,6 @@ technologies, such as screen readers, has been detected. See
https://www.chromium.org/developers/design-documents/accessibility for more
details.

**[Deprecated](modernization/property-updates.md)**

### `app.setAccessibilitySupportEnabled(enabled)` _macOS_ _Windows_

* `enabled` Boolean - Enable or disable [accessibility tree](https://developers.google.com/web/fundamentals/accessibility/semantics-builtin/the-accessibility-tree) rendering
Expand All @@ -1184,8 +1174,6 @@ This API must be called after the `ready` event is emitted.

**Note:** Rendering accessibility tree can significantly affect the performance of your app. It should not be enabled by default.

**[Deprecated](modernization/property-updates.md)**

### `app.showAboutPanel()`

Show the app's about panel options. These options can be overridden with `app.setAboutPanelOptions(options)`.
Expand Down
6 changes: 0 additions & 6 deletions docs/api/modernization/property-updates.md
Expand Up @@ -45,9 +45,3 @@ The Electron team is currently undergoing an initiative to convert separate gett
* `isMacTemplateImage`
* `SystemPreferences` module
* `appLevelAppearance`
* `webContents` module
* `audioMuted`
* `frameRate`
* `userAgent`
* `zoomFactor`
* `zoomLevel`
30 changes: 24 additions & 6 deletions lib/browser/api/app.ts
@@ -1,7 +1,7 @@
import * as fs from 'fs'
import * as path from 'path'

import { deprecate, Menu } from 'electron'
import { Menu } from 'electron'
import { EventEmitter } from 'events'

const bindings = process.electronBinding('app')
Expand All @@ -17,6 +17,29 @@ let dockMenu: Electron.Menu | null = null
Object.setPrototypeOf(App.prototype, EventEmitter.prototype)
EventEmitter.call(app as any)

// Properties.

const nativeASGetter = app.isAccessibilitySupportEnabled
const nativeASSetter = app.setAccessibilitySupportEnabled
Object.defineProperty(App.prototype, 'accessibilitySupportEnabled', {
get: () => nativeASGetter.call(app),
Copy link
Member

@zcbenz zcbenz Mar 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use get: function () { this.isAccessibilitySupportEnabled() } or
get: () => app.isAccessibilitySupportEnabled()?

One benefit is, if user monkey-patches the method the property would also be patched automatically.

Edit: wrong place.

set: (enabled) => nativeASSetter.call(app, enabled)
})

const nativeBCGetter = app.getBadgeCount
const nativeBCSetter = app.setBadgeCount
Object.defineProperty(App.prototype, 'badgeCount', {
get: () => nativeBCGetter.call(app),
set: (count) => nativeBCSetter.call(app, count)
})

const nativeNGetter = app.getName
const nativeNSetter = app.setName
Object.defineProperty(App.prototype, 'name', {
get: () => nativeNGetter.call(app),
set: (name) => nativeNSetter.call(app, name)
})

Object.assign(app, {
commandLine: {
hasSwitch: (theSwitch: string) => commandLine.hasSwitch(String(theSwitch)),
Expand Down Expand Up @@ -112,11 +135,6 @@ for (const name of events) {
})
}

// Property Deprecations
deprecate.fnToProperty(App.prototype, 'accessibilitySupportEnabled', '_isAccessibilitySupportEnabled', '_setAccessibilitySupportEnabled')
deprecate.fnToProperty(App.prototype, 'badgeCount', '_getBadgeCount', '_setBadgeCount')
deprecate.fnToProperty(App.prototype, 'name', '_getName', '_setName')

// Wrappers for native classes.
const { DownloadItem } = process.electronBinding('download_item')
Object.setPrototypeOf(DownloadItem.prototype, EventEmitter.prototype)
20 changes: 6 additions & 14 deletions shell/browser/api/electron_api_app.cc
Expand Up @@ -1410,8 +1410,8 @@ void App::BuildPrototype(v8::Isolate* isolate,
base::BindRepeating(&Browser::GetVersion, browser))
.SetMethod("setVersion",
base::BindRepeating(&Browser::SetVersion, browser))
.SetMethod("_getName", base::BindRepeating(&Browser::GetName, browser))
.SetMethod("_setName", base::BindRepeating(&Browser::SetName, browser))
.SetMethod("getName", base::BindRepeating(&Browser::GetName, browser))
.SetMethod("setName", base::BindRepeating(&Browser::SetName, browser))
.SetMethod("isReady", base::BindRepeating(&Browser::is_ready, browser))
.SetMethod("whenReady", base::BindRepeating(&Browser::WhenReady, browser))
.SetMethod("addRecentDocument",
Expand All @@ -1432,20 +1432,15 @@ void App::BuildPrototype(v8::Isolate* isolate,
.SetMethod(
"getApplicationNameForProtocol",
base::BindRepeating(&Browser::GetApplicationNameForProtocol, browser))
.SetMethod("_setBadgeCount",
.SetMethod("setBadgeCount",
base::BindRepeating(&Browser::SetBadgeCount, browser))
.SetMethod("_getBadgeCount",
.SetMethod("getBadgeCount",
base::BindRepeating(&Browser::GetBadgeCount, browser))
.SetMethod("getLoginItemSettings", &App::GetLoginItemSettings)
.SetMethod("setLoginItemSettings",
base::BindRepeating(&Browser::SetLoginItemSettings, browser))
.SetMethod("isEmojiPanelSupported",
base::BindRepeating(&Browser::IsEmojiPanelSupported, browser))
.SetProperty("badgeCount",
base::BindRepeating(&Browser::GetBadgeCount, browser),
base::BindRepeating(&Browser::SetBadgeCount, browser))
.SetProperty("name", base::BindRepeating(&Browser::GetName, browser),
base::BindRepeating(&Browser::SetName, browser))
#if defined(OS_MACOSX)
.SetMethod("hide", base::BindRepeating(&Browser::Hide, browser))
.SetMethod("show", base::BindRepeating(&Browser::Show, browser))
Expand All @@ -1471,9 +1466,6 @@ void App::BuildPrototype(v8::Isolate* isolate,
#if defined(OS_MACOSX) || defined(OS_WIN)
.SetMethod("showEmojiPanel",
base::BindRepeating(&Browser::ShowEmojiPanel, browser))
.SetProperty("accessibilitySupportEnabled",
&App::IsAccessibilitySupportEnabled,
&App::SetAccessibilitySupportEnabled)
#endif
#if defined(OS_WIN)
.SetMethod("setUserTasks",
Expand All @@ -1500,9 +1492,9 @@ void App::BuildPrototype(v8::Isolate* isolate,
.SetMethod("requestSingleInstanceLock", &App::RequestSingleInstanceLock)
.SetMethod("releaseSingleInstanceLock", &App::ReleaseSingleInstanceLock)
.SetMethod("relaunch", &App::Relaunch)
.SetMethod("_isAccessibilitySupportEnabled",
.SetMethod("isAccessibilitySupportEnabled",
&App::IsAccessibilitySupportEnabled)
.SetMethod("_setAccessibilitySupportEnabled",
.SetMethod("setAccessibilitySupportEnabled",
&App::SetAccessibilitySupportEnabled)
.SetMethod("disableHardwareAcceleration",
&App::DisableHardwareAcceleration)
Expand Down
104 changes: 66 additions & 38 deletions spec-main/api-app-spec.ts
Expand Up @@ -83,35 +83,33 @@ describe('app module', () => {
})
})

describe('app.name', () => {
it('returns the name field of package.json', () => {
expect(app.name).to.equal('Electron Test Main')
})
describe('app name APIs', () => {
it('with properties', () => {
it('returns the name field of package.json', () => {
expect(app.name).to.equal('Electron Test Main')
})

it('overrides the name', () => {
expect(app.name).to.equal('Electron Test Main')
app.name = 'test-name'
it('overrides the name', () => {
expect(app.name).to.equal('Electron Test Main')
app.name = 'test-name'

expect(app.name).to.equal('test-name')
app.name = 'Electron Test Main'
expect(app.name).to.equal('test-name')
app.name = 'Electron Test Main'
})
})
})

// TODO(codebytere): remove when propertyification is complete
describe('app.getName()', () => {
it('returns the name field of package.json', () => {
expect(app.getName()).to.equal('Electron Test Main')
})
})
it('with functions', () => {
it('returns the name field of package.json', () => {
expect(app.getName()).to.equal('Electron Test Main')
})

// TODO(codebytere): remove when propertyification is complete
describe('app.setName(name)', () => {
it('overrides the name', () => {
expect(app.getName()).to.equal('Electron Test Main')
app.setName('test-name')
it('overrides the name', () => {
expect(app.getName()).to.equal('Electron Test Main')
app.setName('test-name')

expect(app.getName()).to.equal('test-name')
app.setName('Electron Test Main')
expect(app.getName()).to.equal('test-name')
app.setName('Electron Test Main')
})
})
})

Expand Down Expand Up @@ -549,20 +547,42 @@ describe('app module', () => {
after(() => { app.badgeCount = 0 })

describe('on supported platform', () => {
it('sets a badge count', function () {
if (platformIsNotSupported) return this.skip()
it('with properties', () => {
it('sets a badge count', function () {
if (platformIsNotSupported) return this.skip()

app.badgeCount = expectedBadgeCount
expect(app.badgeCount).to.equal(expectedBadgeCount)
})
})

app.badgeCount = expectedBadgeCount
expect(app.badgeCount).to.equal(expectedBadgeCount)
it('with functions', () => {
it('sets a badge count', function () {
if (platformIsNotSupported) return this.skip()

app.setBadgeCount(expectedBadgeCount)
expect(app.getBadgeCount()).to.equal(expectedBadgeCount)
})
})
})

describe('on unsupported platform', () => {
it('does not set a badge count', function () {
if (platformIsSupported) return this.skip()
it('with properties', () => {
it('does not set a badge count', function () {
if (platformIsSupported) return this.skip()

app.badgeCount = 9999
expect(app.badgeCount).to.equal(0)
app.badgeCount = 9999
expect(app.badgeCount).to.equal(0)
})
})

it('with functions', () => {
it('does not set a badge count)', function () {
if (platformIsSupported) return this.skip()

app.setBadgeCount(9999)
expect(app.getBadgeCount()).to.equal(0)
})
})
})
})
Expand Down Expand Up @@ -650,15 +670,23 @@ describe('app module', () => {
})
})

describe('accessibilitySupportEnabled property', () => {
if (process.platform === 'linux') return
ifdescribe(process.platform !== 'linux')('accessibilitySupportEnabled property', () => {
it('with properties', () => {
it('can set accessibility support enabled', () => {
expect(app.accessibilitySupportEnabled).to.eql(false)

it('returns whether the Chrome has accessibility APIs enabled', () => {
expect(app.accessibilitySupportEnabled).to.be.a('boolean')
app.accessibilitySupportEnabled = true
expect(app.accessibilitySupportEnabled).to.eql(true)
})
})

// TODO(codebytere): remove when propertyification is complete
expect(app.isAccessibilitySupportEnabled).to.be.a('function')
expect(app.setAccessibilitySupportEnabled).to.be.a('function')
it('with functions', () => {
it('can set accessibility support enabled', () => {
expect(app.isAccessibilitySupportEnabled()).to.eql(false)

app.setAccessibilitySupportEnabled(true)
expect(app.isAccessibilitySupportEnabled()).to.eql(true)
})
})
})

Expand Down