Skip to content
This repository has been archived by the owner on Feb 25, 2023. It is now read-only.

Commit

Permalink
add clipboard monitor to search page
Browse files Browse the repository at this point in the history 
Related to issue #262 about APIs
  • Loading branch information
@siikamiika
siikamiika committed Oct 26, 2019
1 parent bebd70b commit 704864b
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 2 deletions.
44 changes: 43 additions & 1 deletion ext/bg/js/search.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,13 @@ class DisplaySearch extends Display {
this.search = document.querySelector('#search');
this.query = document.querySelector('#query');
this.intro = document.querySelector('#intro');
this.clipboardMonitorCheck = document.querySelector('#clipboard-monitor');

this.introVisible = true;
this.introAnimationTimer = null;

this.clipboardMonitorIntervalId = null;
this.clipboardPrevText = null;
}

static create() {
Expand All @@ -57,10 +62,20 @@ class DisplaySearch extends Display {

window.wanakana.bind(this.query);
}
if (this.clipboardMonitorCheck !== null) {
this.clipboardMonitorCheck.addEventListener('change', (e) => {
if (e.target.checked) {
this.startClipboardMonitor();
} else {
this.stopClipboardMonitor();
}
});
}

window.addEventListener('popstate', (e) => this.onPopState(e));

this.updateSearchButton();
this.initClipboardMonitor();
} catch (e) {
this.onError(e);
}
Expand Down Expand Up @@ -93,7 +108,9 @@ class DisplaySearch extends Display {
return;
}

e.preventDefault();
if (e) {
e.preventDefault();
}

const query = this.query.value;
const queryString = query.length > 0 ? `?query=${encodeURIComponent(query)}` : '';
Expand Down Expand Up @@ -182,6 +199,31 @@ class DisplaySearch extends Display {
}
}

initClipboardMonitor() {
// ignore copy from search page
window.addEventListener('copy', (e) => {
this.clipboardPrevText = document.getSelection().toString().trim();
});
}

startClipboardMonitor() {
this.clipboardMonitorIntervalId = setInterval(async () => {
const curText = (await navigator.clipboard.readText()).trim();
if (curText && (curText !== this.clipboardPrevText)) {
this.query.value = curText;
this.onSearch();
this.clipboardPrevText = curText;
}
}, 100);
}

stopClipboardMonitor() {
if (this.clipboardMonitorIntervalId) {
clearInterval(this.clipboardMonitorIntervalId);
this.clipboardMonitorIntervalId = null;
}
}

getOptionsContext() {
return this.optionsContext;
}
Expand Down
9 changes: 8 additions & 1 deletion ext/bg/search.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,14 @@ <h1>Yomichan Search</h1>
<p style="margin-bottom: 0;">Search your installed dictionaries by entering a Japanese expression into the field below.</p>
</div>

<form class="input-group" style="padding-top: 10px;">
<div class="input-group" style="padding-top: 10px; font-size: 20px;">
<span title="Enable clipboard monitor" class="input-group-text">
<label for="clipboard-monitor"><span class="glyphicon glyphicon-paste"></span></label>
<input type="checkbox" id="clipboard-monitor" />
</span>
</div>

<form class="input-group">
<input type="text" class="form-control" placeholder="Search for..." id="query" autofocus>
<span class="input-group-btn">
<input type="submit" class="btn btn-default form-control" id="search" value="Search">
Expand Down
1 change: 1 addition & 0 deletions ext/manifest.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
"permissions": [
"<all_urls>",
"storage",
"clipboardRead",

This comment has been minimized.

Sign in to view

Copy link
@toasted-nutbread

toasted-nutbread Oct 26, 2019

Collaborator

Would it be worth specifying clipboardRead as an optional permission, and only request it post-install? May run into the same issue I mentioned in #256. IMO it may be better if the extension requests less permissions up front, as users might be suspicious of the extension being able to freely read the clipboard.

This comment has been minimized.

Sign in to view

Copy link
@siikamiika

siikamiika Oct 26, 2019

Author Collaborator

I know some people are quite paranoid when it comes to extension permission even on FOSS software, and this could scare them away. I still agree with the point that less permissions is better, especially since clipboard can contain very sensitive information like passwords. I'll see later if it causes any issues when the permission is requested after enabling clipboard monitor.

This comment has been minimized.

Sign in to view

Copy link
@siikamiika

siikamiika Oct 27, 2019

Author Collaborator

I made clipboardRead an optional permission on master. It didn't seem to cause any problems, so I'm wondering if the issues you mentioned were caused by not adding the corresponding optional_permssions entry.

However, another problem arised. It seems that Chrome doesn't allow using navigator.clipboard.readText in the background, and I opted to copy the hack that Clipboard Inserter uses. While Clipboard Inserter works on Firefox, I could only get the copied version work on Chrome. Now my draft checks which browser is used and chooses the method to use based on that, but I'll try to remove the browser check before making a commit.

The patch:

diff --git a/ext/bg/background.html b/ext/bg/background.html
index 194d4a4..30b3db4 100644
--- a/ext/bg/background.html
+++ b/ext/bg/background.html
@@ -5,6 +5,8 @@
         <meta name="viewport" content="width=device-width,initial-scale=1" />
     </head>
     <body>
+        <div id="clipboard-paste-target" contenteditable="true"></div>
+
         <script src="/mixed/lib/dexie.min.js"></script>
         <script src="/mixed/lib/handlebars.min.js"></script>
         <script src="/mixed/lib/jszip.min.js"></script>
diff --git a/ext/bg/js/api.js b/ext/bg/js/api.js
index 999ea33..87580b8 100644
--- a/ext/bg/js/api.js
+++ b/ext/bg/js/api.js
@@ -401,3 +401,11 @@ async function apiFocusTab(tab) {
         // Edge throws exception for no reason here.
     }
 }
+
+async function apiClipboardGet() {
+    const clipboardPasteTarget = document.querySelector('#clipboard-paste-target');
+    clipboardPasteTarget.innerText = '';
+    clipboardPasteTarget.focus();
+    document.execCommand('paste');
+    return clipboardPasteTarget.innerText.trim();
+}
diff --git a/ext/bg/js/backend.js b/ext/bg/js/backend.js
index 23d876f..dacc297 100644
--- a/ext/bg/js/backend.js
+++ b/ext/bg/js/backend.js
@@ -187,7 +187,9 @@ Backend.messageHandlers = {
     forward: ({action, params}, sender) => apiForward(action, params, sender),
     frameInformationGet: (params, sender) => apiFrameInformationGet(sender),
     injectStylesheet: ({css}, sender) => apiInjectStylesheet(css, sender),
-    getEnvironmentInfo: () => apiGetEnvironmentInfo()
+    getEnvironmentInfo: () => apiGetEnvironmentInfo(),
+    clipboardGet: () => apiClipboardGet(),
+    getBrowser: () => apiGetBrowser()
 };
 
 window.yomichan_backend = new Backend();
diff --git a/ext/bg/js/search.js b/ext/bg/js/search.js
index a09ca82..2a24676 100644
--- a/ext/bg/js/search.js
+++ b/ext/bg/js/search.js
@@ -235,7 +235,13 @@ class DisplaySearch extends Display {
 
     startClipboardMonitor() {
         this.clipboardMonitorIntervalId = setInterval(async () => {
-            const curText = (await navigator.clipboard.readText()).trim();
+            let curText = null;
+            // TODO get rid of this and figure out why apiClipboardGet doesn't work on Firefox
+            if (await apiGetBrowser() === 'firefox') {
+                curText = (await navigator.clipboard.readText()).trim();
+            } else {
+                curText = await apiClipboardGet();
+            }
             if (curText && (curText !== this.clipboardPrevText)) {
                 if (this.isWanakanaEnabled()) {
                     this.query.value = window.wanakana.toKana(curText);
diff --git a/ext/fg/js/api.js b/ext/fg/js/api.js
index b0746b8..c46f5b6 100644
--- a/ext/fg/js/api.js
+++ b/ext/fg/js/api.js
@@ -72,3 +72,11 @@ function apiInjectStylesheet(css) {
 function apiGetEnvironmentInfo() {
     return utilInvoke('getEnvironmentInfo');
 }
+
+function apiClipboardGet() {
+    return utilInvoke('clipboardGet');
+}
+
+function apiGetBrowser() {
+    return utilInvoke('getBrowser');
+}

This comment has been minimized.

Sign in to view

Copy link
@toasted-nutbread

toasted-nutbread Oct 27, 2019

Collaborator

I'm wondering if the issues you mentioned were caused by not adding the corresponding optional_permssions entry.

Even if you include unlimitedStorage as an optional_permissions entry, it's not allowed. That's why that change needed to be a non-optional permission, which has potential to trigger a warning to the user.

Chrome error:
The optional permissions API does not support 'unlimitedStorage'.

Firefox error:
Type error for parameter permissions (Error processing permissions.0: Value "unlimitedStorage" must either: be one of ["clipboardRead", "clipboardWrite", "geolocation", "idle", "notifications"], be one of ["bookmarks"], be one of ["find"], be one of ["history"], be one of ["menus.overrideContext"], be one of ["search"], be one of ["activeTab", "tabs", "tabHide"], be one of ["topSites"], be one of ["browserSettings"], be one of ["cookies"], be one of ["downloads", "downloads.open"], be one of ["webNavigation"], or be one of ["webRequest", "webRequestBlocking"]) for permissions.request.

This comment has been minimized.

Sign in to view

Copy link
@siikamiika

siikamiika Oct 27, 2019

Author Collaborator

Ok, that's too bad.

There seems to be some discussion about adding that support to Firefox in this bug, but I didn't find anything at bugs.chromium.org.
"clipboardWrite",
"unlimitedStorage"
],
Expand Down

0 comments on commit 704864b

Please sign in to comment.