diff --git a/app/browser/menu.js b/app/browser/menu.js index 891872eb010..b5dfe458787 100644 --- a/app/browser/menu.js +++ b/app/browser/menu.js @@ -97,19 +97,19 @@ const createFileSubmenu = () => { } }, { label: locale.translation('share'), - visible: false - /* submenu: [ - {label: 'Email Page Link...'}, + CommonMenu.simpleShareActiveTabMenuItem('emailPageLink', 'email', 'CmdOrCtrl+Shift+I'), CommonMenu.separatorMenuItem, - {label: 'Tweet Page...'}, - {label: 'Share on Facebook...'}, - {label: 'More...'} + CommonMenu.simpleShareActiveTabMenuItem('tweetPageLink', 'twitter'), + CommonMenu.simpleShareActiveTabMenuItem('facebookPageLink', 'facebook'), + CommonMenu.simpleShareActiveTabMenuItem('pinterestPageLink', 'pinterest'), + CommonMenu.simpleShareActiveTabMenuItem('googlePlusPageLink', 'googlePlus'), + CommonMenu.simpleShareActiveTabMenuItem('linkedInPageLink', 'linkedIn'), + CommonMenu.simpleShareActiveTabMenuItem('bufferPageLink', 'buffer'), + CommonMenu.simpleShareActiveTabMenuItem('redditPageLink', 'reddit') ] - */ }, // Move inside share menu when it's enabled - CommonMenu.emailPageLinkMenuItem(), CommonMenu.separatorMenuItem, CommonMenu.printMenuItem() ] diff --git a/app/browser/reducers/shareReducer.js b/app/browser/reducers/shareReducer.js index 0e0c76c7cfe..91b1df8e30c 100644 --- a/app/browser/reducers/shareReducer.js +++ b/app/browser/reducers/shareReducer.js @@ -6,13 +6,13 @@ const appConstants = require('../../../js/constants/appConstants') const {makeImmutable} = require('../../common/state/immutableUtil') -const {emailActiveTab} = require('../share') +const {simpleShareActiveTab} = require('../share') const shareReducer = (state, action) => { action = makeImmutable(action) switch (action.get('actionType')) { - case appConstants.APP_EMAIL_ACTIVE_TAB_REQUESTED: - state = emailActiveTab(state, action.get('windowId')) + case appConstants.APP_SIMPLE_SHARE_ACTIVE_TAB_REQUESTED: + state = simpleShareActiveTab(state, action.get('windowId'), action.get('shareType')) break } return state diff --git a/app/browser/share.js b/app/browser/share.js index 3e74b877319..99f78ef8035 100644 --- a/app/browser/share.js +++ b/app/browser/share.js @@ -2,17 +2,55 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at http://mozilla.org/MPL/2.0/. */ +const assert = require('assert') const tabState = require('../common/state/tabState') const {shell} = require('electron') +const tabs = require('./tabs') -const emailActiveTab = (state, windowId) => { +const templateUrls = { + email: 'mailto:?subject={title}&body={url}', + facebook: 'https://www.facebook.com/sharer.php?u={url}', + pinterest: 'https://pinterest.com/pin/create/bookmarklet/?url={url}&description={title}', + twitter: 'https://twitter.com/intent/tweet?url={url}&text={title}&hashtags=brave', + googlePlus: 'https://plus.google.com/share?url={url}', + linkedIn: 'https://www.linkedin.com/shareArticle?url={url}&title={title}', + buffer: 'https://buffer.com/add?text={title}&url={url}', + digg: 'http://digg.com/submit?url={url}&title={title}', + reddit: 'https://reddit.com/submit?url={url}&title={title}' +} + +const validateShareType = (shareType) => + assert(templateUrls[shareType], 'The specified shareType is not recognized') + +/** + * Performs a simple share operation for the active tab in the specified window. + * + * @param {number} windowId - The window for the active tab to use + * @param {string} shareType - The template to use, see the property key names in templateUrls above. + */ +const simpleShareActiveTab = (state, windowId, shareType) => { const tabValue = tabState.getActiveTabValue(state, windowId) - shell.openExternal( - `mailto:?subject=${encodeURIComponent(tabValue.get('title') || '')}&body=${encodeURIComponent(tabValue.get('url'))}` - ) + const encodedTitle = encodeURIComponent(tabValue.get('title') || '') + const encodedUrl = encodeURIComponent(tabValue.get('url')) + + validateShareType(shareType) + const templateUrl = templateUrls[shareType] + + const url = templateUrl + .replace('{url}', encodedUrl) + .replace('{title}', encodedTitle) + + if (shareType === 'email') { + shell.openExternal(url) + } else { + tabs.create({ + url + }) + } return state } module.exports = { - emailActiveTab + simpleShareActiveTab, + templateUrls } diff --git a/app/common/commonMenu.js b/app/common/commonMenu.js index b4b8cc2d2b0..a4a97c7f4e4 100644 --- a/app/common/commonMenu.js +++ b/app/common/commonMenu.js @@ -148,12 +148,12 @@ module.exports.printMenuItem = () => { } } -module.exports.emailPageLinkMenuItem = () => { +module.exports.simpleShareActiveTabMenuItem = (l10nId, type, accelerator) => { return { - label: locale.translation('emailPageLink'), - accelerator: 'CmdOrCtrl+Shift+I', + label: locale.translation(l10nId), + accelerator, click: function (item, focusedWindow) { - appActions.emailActiveTabRequested(getCurrentWindowId()) + appActions.simpleShareActiveTabRequested(getCurrentWindowId(), type) } } } diff --git a/app/extensions/brave/locales/en-US/menu.properties b/app/extensions/brave/locales/en-US/menu.properties index 43110465511..84a42c1d0b5 100644 --- a/app/extensions/brave/locales/en-US/menu.properties +++ b/app/extensions/brave/locales/en-US/menu.properties @@ -81,6 +81,13 @@ newWindow=New Window reopenLastClosedTab=Reopen Last Closed Tab print=Print… emailPageLink=Email Page Link… +tweetPageLink=Tweet Page… +facebookPageLink=Share on Facebook… +pinterestPageLink=Share on Pinterest… +googlePlusPageLink=Share on Google+… +linkedInPageLink=Share on LinkedIn… +bufferPageLink=Share on Buffer… +redditPageLink=Share on Reddit… findOnPage=Find on Page… find=Find… checkForUpdates=Check for Updates… diff --git a/app/locale.js b/app/locale.js index 5b00ce9b5bf..964a9d3f4d5 100644 --- a/app/locale.js +++ b/app/locale.js @@ -154,6 +154,13 @@ var rendererIdentifiers = function () { 'reopenLastClosedTab', 'print', 'emailPageLink', + 'tweetPageLink', + 'facebookPageLink', + 'pinterestPageLink', + 'googlePlusPageLink', + 'linkedInPageLink', + 'bufferPageLink', + 'redditPageLink', 'findOnPage', 'find', 'checkForUpdates', diff --git a/js/actions/appActions.js b/js/actions/appActions.js index a521622ffaa..fcbcd6c31fa 100644 --- a/js/actions/appActions.js +++ b/js/actions/appActions.js @@ -151,11 +151,13 @@ const appActions = { /** * A request for a URL email share occurred * @param {number} windowId - the window ID to use for the active tab + * @param {string} shareType - The type of share to do, must be one of: "email", "facebook", "pinterest", "twitter", "googlePlus", "linkedIn", "buffer", "reddit", or "digg" */ - emailActiveTabRequested: function (windowId) { + simpleShareActiveTabRequested: function (windowId, shareType) { AppDispatcher.dispatch({ - actionType: appConstants.APP_EMAIL_ACTIVE_TAB_REQUESTED, - windowId + actionType: appConstants.APP_SIMPLE_SHARE_ACTIVE_TAB_REQUESTED, + windowId, + shareType }) }, diff --git a/js/constants/appConstants.js b/js/constants/appConstants.js index 05c57d7cb5e..c0ec32bc11d 100644 --- a/js/constants/appConstants.js +++ b/js/constants/appConstants.js @@ -70,7 +70,7 @@ const appConstants = { APP_MAYBE_CREATE_TAB_REQUESTED: _, APP_LOAD_URL_REQUESTED: _, APP_LOAD_URL_IN_ACTIVE_TAB_REQUESTED: _, - APP_EMAIL_ACTIVE_TAB_REQUESTED: _, + APP_SIMPLE_SHARE_ACTIVE_TAB_REQUESTED: _, APP_NEW_WEB_CONTENTS_ADDED: _, APP_TAB_DESTROYED: _, APP_SET_MENUBAR_TEMPLATE: _, diff --git a/test/unit/app/browser/reducers/shareReducerTest.js b/test/unit/app/browser/reducers/shareReducerTest.js index 9e6e36d48f3..c2c7fe2d313 100644 --- a/test/unit/app/browser/reducers/shareReducerTest.js +++ b/test/unit/app/browser/reducers/shareReducerTest.js @@ -18,7 +18,7 @@ describe('shareReducer', function () { }) mockery.registerMock('electron', fakeElectron) this.shareStub = { - emailActiveTab: sinon.stub() + simpleShareActiveTab: sinon.stub() } mockery.registerMock('../share', this.shareStub) shareReducer = require('../../../../../app/browser/reducers/shareReducer') @@ -28,14 +28,15 @@ describe('shareReducer', function () { mockery.disable() }) - describe('APP_EMAIL_ACTIVE_TAB_REQUESTED', function () { + describe('APP_SIMPLE_SHARE_ACTIVE_TAB_REQUESTED', function () { before(function () { this.state = Immutable.Map() this.windowId = 2 - this.newState = shareReducer(this.state, {actionType: appConstants.APP_EMAIL_ACTIVE_TAB_REQUESTED, windowId: this.windowId}) + this.shareType = 'email' + this.newState = shareReducer(this.state, {actionType: appConstants.APP_SIMPLE_SHARE_ACTIVE_TAB_REQUESTED, windowId: this.windowId, shareType: this.shareType}) }) - it('calls emailActiveTab once with the correct args', function () { - const callCount = this.shareStub.emailActiveTab.withArgs(this.state, this.windowId).callCount + it('calls simpleShareActiveTab once with the correct args', function () { + const callCount = this.shareStub.simpleShareActiveTab.withArgs(this.state, this.windowId, this.shareType).callCount assert.equal(callCount, 1) }) }) diff --git a/test/unit/app/browser/shareTest.js b/test/unit/app/browser/shareTest.js index 94cd2fcd217..b6f7bbc0adc 100644 --- a/test/unit/app/browser/shareTest.js +++ b/test/unit/app/browser/shareTest.js @@ -16,6 +16,11 @@ describe('share API', function () { useCleanCache: true }) mockery.registerMock('electron', fakeElectron) + + this.mockTabs = { + create: sinon.stub() + } + mockery.registerMock('./tabs', this.mockTabs) share = require('../../../../app/browser/share') }) @@ -23,7 +28,7 @@ describe('share API', function () { mockery.disable() }) - describe('emailActiveTab', function () { + describe('simpleShareActiveTab', function () { before(function () { this.state = Immutable.fromJS({ tabs: [{ @@ -50,18 +55,41 @@ describe('share API', function () { }) afterEach(function () { this.openExternalSpy.reset() + this.mockTabs.create.reset() }) - it('calls openExternal with the correct args', function () { - share.emailActiveTab(this.state, 2) - const expectedUrl = 'mailto:?subject=title%202&body=https%3A%2F%2Fwww.brave.com%2F2' - const callCount = this.openExternalSpy.withArgs(expectedUrl).callCount - assert.equal(callCount, 1) + describe('email', function () { + it('calls openExternal with the correct args', function () { + share.simpleShareActiveTab(this.state, 2, 'email') + const expectedUrl = 'mailto:?subject=title%202&body=https%3A%2F%2Fwww.brave.com%2F2' + const callCount = this.openExternalSpy.withArgs(expectedUrl).callCount + assert.equal(callCount, 1) + assert.equal(this.mockTabs.create.withArgs(expectedUrl).callCount, 0) + }) + it('takes active tab windowId into consideration', function () { + share.simpleShareActiveTab(this.state, 5, 'email') + const expectedUrl = 'mailto:?subject=title%205&body=https%3A%2F%2Fwww.brave.com%2F5' + const callCount = this.openExternalSpy.withArgs(expectedUrl).callCount + assert.equal(callCount, 1) + assert.equal(this.mockTabs.create.withArgs(expectedUrl).callCount, 0) + }) }) - it('takes active tab windowId into consideration', function () { - share.emailActiveTab(this.state, 5) - const expectedUrl = 'mailto:?subject=title%205&body=https%3A%2F%2Fwww.brave.com%2F5' - const callCount = this.openExternalSpy.withArgs(expectedUrl).callCount - assert.equal(callCount, 1) + describe('other', function () { + it('calls createTabRequested with correct args', function () { + Object.entries(share.templateUrls).forEach(([shareType, template]) => { + if (shareType === 'email') { + return + } + share.simpleShareActiveTab(this.state, 5, shareType) + const expectedUrl = template + .replace('{url}', 'https%3A%2F%2Fwww.brave.com%2F5') + .replace('{title}', 'title%205') + assert.equal(this.openExternalSpy.withArgs({url: expectedUrl}).callCount, 0) + assert.equal(this.mockTabs.create.callCount, 1) + assert.equal(this.mockTabs.create.args[0][0].url, expectedUrl) + this.openExternalSpy.reset() + this.mockTabs.create.reset() + }) + }) }) }) }) diff --git a/test/unit/app/common/commonMenuTest.js b/test/unit/app/common/commonMenuTest.js index 16001312501..8d98a962600 100644 --- a/test/unit/app/common/commonMenuTest.js +++ b/test/unit/app/common/commonMenuTest.js @@ -102,9 +102,17 @@ describe('Common menu module unit tests', function () { }) }) - describe('emailPageLinkMenuItem', function () { - it('has the expected defaults set', function () { - checkExpectedDefaults(commonMenu.emailPageLinkMenuItem) + describe('simpleShareActiveTabMenuItem', function () { + it('has the expected defaults set', function () { + checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'emailPageLink', 'email', 'CmdOrCtrl+Shift+I'), true) + checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'tweetPageLink', 'twitter'), false) + checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'facebookPageLink', 'facebook'), false) + checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'pinterestPageLink', 'pinterest'), false) + checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'googlePlusPageLink', 'googlePlus'), false) + checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'linkedInPageLink', 'linkedIn'), false) + checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'bufferPageLink', 'buffer'), false) + checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'redditPageLink', 'reddit'), false) + checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'diggPageLink', 'digg'), false) }) }) diff --git a/test/unit/lib/fakeElectron.js b/test/unit/lib/fakeElectron.js index 9be0b188294..8cdbe302416 100644 --- a/test/unit/lib/fakeElectron.js +++ b/test/unit/lib/fakeElectron.js @@ -59,6 +59,11 @@ const fakeElectron = { }, moveItemToTrash: function () { } + }, + session: { + defaultSession: { + partition: 'default' + } } }