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
Add GM.notification, GM.openInTab and GM.setClipboard #2590
Conversation
src/bg/api-provider-source.js
Outdated
@@ -172,4 +188,69 @@ function GM_xmlHttpRequest(d) { | |||
// TODO: Return an object which can be `.abort()`ed. | |||
} | |||
|
|||
function GM_openInTab(url, openInBackground) { | |||
let _url; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove underscore prefixes.
src/bg/api-provider-source.js
Outdated
_url = new URL(url, location.href); | ||
} catch(e) { | ||
throw new Error( | ||
'GM.openInTab: Could not understand the URL: ' + url |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this split to a second line?
event.clipboardData.setData('text/plain', text); | ||
} | ||
|
||
document.addEventListener('copy', onCopy, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will add a listener to the document every time setClipboard is called?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for my bad english, Anthony. If I don't remove the listener and a page script calls document.execCommand('copy') from event handler, onCopy() will be executed. I know it's a dirty solution, but it isn't possible to write to the clipboard on Firefox from background scripts.
src/bg/api-provider-source.js
Outdated
|
||
/* | ||
* GM_notification(details, ondone) | ||
* GM_notification(text, title, image, onclick) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this comment mean? I think you want some prose to explain the meaning of these two lines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll remove this comment. The function is similar to GM_notification on Tampermonkey (https://tampermonkey.net/documentation.php#GM_notification), but there is no support to hightlight tab (it is not possible to do that on Firefox right now).
src/bg/api-provider-source.js
Outdated
} | ||
|
||
typeof opt.title !== 'string' && (opt.title = 'Greasemonkey'); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What purpose does this blank line serve?
src/bg/api-provider-source.js
Outdated
throw new Error('GM.notification: "text" must be a string'); | ||
} | ||
|
||
typeof opt.title !== 'string' && (opt.title = 'Greasemonkey'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a fan of this, why not:
if (typeof opt.title != 'string') opt.title = 'Greasemonkey';
It's no longer, and easier to read.
src/bg/api-provider-source.js
Outdated
typeof opt.image !== 'string' && (opt.image = 'skin/icon32.png'); | ||
|
||
let port = chrome.runtime.connect({name: 'UserScriptNotification'}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the significance of this blank line?
src/bg/api-provider-source.js
Outdated
typeof opt[msgType] === 'function' && opt[msgType](); | ||
}); | ||
|
||
port.postMessage({ name: 'create', details: { title: opt.title, text: opt.text, image: opt.image } }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep all lines under 80 columns; try to follow formatting used throughout the rest of the code base.
|
||
port.onMessage.addListener(msg => { | ||
switch (msg.name) { | ||
case 'create': createNotification(msg.details, port); break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One statement per line please.
(In some cases this makes sense, but not when followed by another branch with a different style.)
src/bg/on-user-script-open-in-tab.js
Outdated
}); | ||
} | ||
|
||
window.onApiOpenInTab = onApiOpenInTab; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the entire file, so you can skip the "private" wrapper and just put the public method directly in the global scope.
GM.openInTab(url, openInBackground = true)
GM.setClipboard(text)
GM.notification(details, ondone) / GM.notification(text, title, image, onclick)
Test script:
https://gist.githubusercontent.com/xor10/973c4fb17671dac987676e1b11d136ab/raw/39521431d17d61a7d8a161e2f4dec5668d9f7991/test_api.user.js