From f203bdb7ded431cd1e94cd42d94da5b94aff5c46 Mon Sep 17 00:00:00 2001 From: Jared Hirsch Date: Tue, 30 Oct 2018 11:20:45 -0700 Subject: [PATCH] Fixes #5089 - Screenshots keyboard shortcut TIL Can't specify 'option' as a webextension hot key. Just go with ctrl + shift + s, which will be command + shift + s on mac. --- webextension/background/main.js | 64 +++++++++++----------- webextension/background/startBackground.js | 14 +++++ webextension/manifest.json.template | 8 +++ webextension/onboarding/slides.js | 5 +- 4 files changed, 57 insertions(+), 34 deletions(-) diff --git a/webextension/background/main.js b/webextension/background/main.js index 0ecf047b7c..50d96dc19b 100644 --- a/webextension/background/main.js +++ b/webextension/background/main.js @@ -83,36 +83,7 @@ this.main = (function() { // This is called by startBackground.js, where is registered as a click // handler for the webextension page action. exports.onClicked = catcher.watchFunction((tab) => { - catcher.watchPromise(hasSeenOnboarding.then(onboarded => { - if (shouldOpenMyShots(tab.url)) { - if (!onboarded) { - catcher.watchPromise(analytics.refreshTelemetryPref().then(() => { - sendEvent("goto-onboarding", "selection-button", {incognito: tab.incognito}); - return forceOnboarding(); - })); - return; - } - catcher.watchPromise(analytics.refreshTelemetryPref().then(() => { - sendEvent("goto-myshots", "about-newtab", {incognito: tab.incognito}); - })); - catcher.watchPromise( - auth.maybeLogin() - .then(() => browser.tabs.update({url: backend + "/shots"}))); - } else { - catcher.watchPromise( - toggleSelector(tab) - .then(active => { - const event = active ? "start-shot" : "cancel-shot"; - sendEvent(event, "toolbar-button", {incognito: tab.incognito}); - }, (error) => { - if ((!onboarded) && error.popupMessage === "UNSHOOTABLE_PAGE") { - sendEvent("goto-onboarding", "selection-button", {incognito: tab.incognito}); - return forceOnboarding(); - } - throw error; - })); - } - })); + _startShotFlow(tab, "toolbar-button"); }); function forceOnboarding() { @@ -120,6 +91,23 @@ this.main = (function() { } exports.onClickedContextMenu = catcher.watchFunction((info, tab) => { + _startShotFlow(tab, "context-menu"); + }); + + exports.onCommand = catcher.watchFunction((tab) => { + _startShotFlow(tab, "keyboard-shortcut"); + }); + + const _openMyShots = (inputType) => { + catcher.watchPromise(analytics.refreshTelemetryPref().then(() => { + sendEvent("goto-myshots", inputType, {incognito: tab.incognito}); + })); + catcher.watchPromise( + auth.maybeLogin() + .then(() => browser.tabs.update({url: backend + "/shots"}))); + }; + + const _startShotFlow = (tab, inputType) => { catcher.watchPromise(hasSeenOnboarding.then(onboarded => { if (!tab) { // Not in a page/tab context, ignore @@ -127,7 +115,8 @@ this.main = (function() { } if (!urlEnabled(tab.url)) { if (!onboarded) { - sendEvent("goto-onboarding", "selection-button", {incognito: tab.incognito}); + // Updated generic "selection-button" event data to inputType + sendEvent("goto-onboarding", inputType, {incognito: tab.incognito}); forceOnboarding(); return; } @@ -135,13 +124,22 @@ this.main = (function() { popupMessage: "UNSHOOTABLE_PAGE", }); return; + } else if (shouldOpenMyShots(tab.url)) { + _openMyShots(inputType); + return; } // No need to catch() here because of watchPromise(). // eslint-disable-next-line promise/catch-or-return toggleSelector(tab) - .then(() => sendEvent("start-shot", "context-menu", {incognito: tab.incognito})); + .then((active) => { + let event = "start-shot"; + if (inputType !== "context-menu") { + event = active ? "start-shot" : "cancel-shot"; + } + sendEvent(event, inputType, {incognito: tab.incognito}) + }); })); - }); + }; function urlEnabled(url) { if (shouldOpenMyShots(url)) { diff --git a/webextension/background/startBackground.js b/webextension/background/startBackground.js index 0cdedb22db..3e8c685cd2 100644 --- a/webextension/background/startBackground.js +++ b/webextension/background/startBackground.js @@ -54,6 +54,20 @@ this.startBackground = (function() { browser.experiments.screenshots.initLibraryButton(); + browser.commands.onCommand.addListener((cmd) => { + if (cmd !== "take-screenshot") { + return; + } + loadIfNecessary().then(() => { + browser.tabs.query({currentWindow: true, active: true}).then((tabs) => { + const activeTab = tabs[0]; + main.onCommand(activeTab); + }); + }).catch((error) => { + console.error("Error toggling Screenshots via keyboard shortcut: ", error); + }); + }); + browser.runtime.onMessage.addListener((req, sender, sendResponse) => { loadIfNecessary().then(() => { return communication.onMessage(req, sender, sendResponse); diff --git a/webextension/manifest.json.template b/webextension/manifest.json.template index 527a8afcd0..5adb9ba3fb 100644 --- a/webextension/manifest.json.template +++ b/webextension/manifest.json.template @@ -18,6 +18,14 @@ "background/startBackground.js" ] }, + "commands": { + "take-screenshot": { + "suggested_key": { + "default": "Ctrl+Shift+S" + }, + "description": "Open the Firefox Screenshots UI" + } + }, "content_scripts": [ { "matches": ["http://localhost/*"], diff --git a/webextension/onboarding/slides.js b/webextension/onboarding/slides.js index 9ef247103a..70a0be97a9 100644 --- a/webextension/onboarding/slides.js +++ b/webextension/onboarding/slides.js @@ -53,10 +53,13 @@ this.slides = (function() { doc.documentElement.lang = browser.i18n.getMessage("@@ui_locale"); localizeText(doc); activateSlide(doc); + // Give the DOM a moment to settle before applying focus + setTimeout(() => { + iframe.contentWindow.focus(); + }); resolve(); }), {once: true}); document.body.appendChild(iframe); - iframe.focus(); window.addEventListener("resize", onResize); }); };