Closed
Bug 1247435
Opened 9 years ago
Closed 8 years ago
Implement browser.runtime.onStartup
Categories
(WebExtensions :: General, defect, P3)
WebExtensions
General
Tracking
(firefox52 verified)
VERIFIED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox52 | --- | verified |
People
(Reporter: steve, Assigned: mattw)
References
Details
(Whiteboard: [runtime][triaged])
Attachments
(2 files)
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:46.0) Gecko/20100101 Firefox/46.0
Build ID: 20160210004006
Steps to reproduce:
Firefox Developer Edition 46.0a2 (2016-02-10)
My extension registers a function in a background page:
chrome.runtime.onStartup.addListener(my_function);
Docs imply onStartup is supported https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/runtime#Chrome_incompatibilities
Actual results:
The browser console reports error
TypeError: chrome.runtime.onStartup is undefined
when the add-on is installed.
onStartup does not appear when chrome.runtime is dumped to string:
{"PlatformOs":{"MAC":"mac","WIN":"win","ANDROID":"android","CROS":"cros","LINUX":"linux","OPENBSD":"openbsd"},"PlatformArch":{"ARM":"arm","X86-32":"x86-32","X86-64":"x86-64"},"RequestUpdateCheckStatus":{"THROTTLED":"throttled","NO_UPDATE":"no_update","UPDATE_AVAILABLE":"update_available"},"OnInstalledReason":{"INSTALL":"install","UPDATE":"update","CHROME_UPDATE":"chrome_update","SHARED_MODULE_UPDATE":"shared_module_update"},"OnRestartRequiredReason":{"APP_UPDATE":"app_update","OS_UPDATE":"os_update","PERIODIC":"periodic"},"onConnect":{},"onMessage":{}}
Expected results:
No console error, the listener should be registered.
It should appear in dump "onStartup":{}
Updated•9 years ago
|
Blocks: webext-runtime
OS: Mac OS X → All
Hardware: x86_64 → All
Summary: WebExtension background page, TypeError: chrome.runtime.onStartup is undefined → Implement browser.runtime.onStartup
Whiteboard: [runtime]
Updated•9 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-webextensions-
Priority: -- → P3
Whiteboard: [runtime] → [runtime] triaged
Updated•9 years ago
|
Whiteboard: [runtime] triaged → [runtime][berlin] triaged
Updated•8 years ago
|
Assignee: nobody → aswan
Whiteboard: [runtime][berlin] triaged → [runtime][triaged]
Assignee | ||
Updated•8 years ago
|
Assignee | ||
Updated•8 years ago
|
Blocks: webext-port-click-and-clean
Updated•8 years ago
|
Blocks: webext-port-google-mail-checker
Updated•8 years ago
|
Assignee: aswan → mwein
Comment 1•8 years ago
|
||
There is a runtime.onStartup implementation, but it is more like a DOMContentLoaded event than Chrome's onStartup event.
http://searchfox.org/mozilla-central/rev/65bed54efcce67cf04a890f7fe253ccdfa6befdc/toolkit/components/extensions/ext-backgroundPage.js#71
// TODO: Right now we run onStartup after the background page
// finishes. See if this is what Chrome does.
runtime.onStartup should be fired if 1) an extension is installed AND 2) the browser starts up for the first time.
It was implemented in Chrome at https://crbug.com/132328
There is another related signal that is currently not implemented in Firefox.
chrome.runtime.onInstalled should be fired when an extension is installed/updated or the browser is updated.
This event is the documented way to register rules for rule-based APIs (contextMenus, declarativeContent, declarativeWebRequest), where the rules are persisted until the extension is reloaded.
We don't support event pages in Firefox at the moment, so it is better to replace what we now call onStartup with onInstalled, and implement onStartup in the way that I described.
Updated•8 years ago
|
Component: WebExtensions: Untriaged → WebExtensions: General
Flags: blocking-webextensions-
Version: 46 Branch → unspecified
The magic actions for youtube is no longer functionallity in the settings page and dos not work with the current new firefox versions like nightly, developer edition, aurora, beta and the stable edition from firefox (I have checked all)!
I can't install this addon (The icon is not shown in the icon bar from firefox) onli in the addons page!
And no strings, booleans and integers are placed in the prefs.js file :(
Or is the addon blocked by the firefox developers?! I don't know :(
I hope the developers fron magic actions for youtube and the developers from mozilla firefox work together to fix the problem with the addon itself and mozilla firefox too.
I have contacted the developers too (For support) for their awesome addon.
ps:The magic actions for youtube is a great addon :)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 5•8 years ago
|
||
mozreview-review |
Comment on attachment 8806546 [details]
Bug 1247435 - Add support for runtime.onStartup
https://reviewboard.mozilla.org/r/89940/#review89618
::: toolkit/components/extensions/ExtensionChild.jsm:164
(Diff revision 2)
> this.viewType = viewType;
> this.uri = uri || extension.baseURI;
>
> this.setContentWindow(contentWindow);
>
> + this.incognito = PrivateBrowsingUtils.isContentWindowPrivate(contentWindow);
setContentWindow should be responsible for setting incognito context.incognito, see 1309610
If all suggested steps in that bug are followed, then you don't need runtime_internal method (because context.incognito is then set both in the child and parent).
Do you think that you can submit a separate patch for that bug (if Kris doesn't have time to fix it)?
Comment 6•8 years ago
|
||
mozreview-review |
Comment on attachment 8806546 [details]
Bug 1247435 - Add support for runtime.onStartup
https://reviewboard.mozilla.org/r/89940/#review89930
::: toolkit/components/extensions/ext-c-runtime.js:20
(Diff revision 2)
> + onStartup: new SingletonEventManager(context, "runtime.onStartup", fire => {
> + let listener = () => {
> + if (!context.incognito) {
> + fire();
> + }
> + };
> + context.childManager.getParentEvent("runtime_internal.onStartup").addListener(listener);
> + return () => {
> + context.childManager.getParentEvent("runtime_internal.onStartup").removeListener(listener);
> + };
> + }).api(),
There shouldn't be any particular need to implement this in the child process. Just initialize the proxy context in the parent process with the incognito property from the child.
::: toolkit/components/extensions/ext-c-runtime.js:22
(Diff revision 2)
>
> onMessage: context.messenger.onMessage("runtime.onMessage"),
>
> + onStartup: new SingletonEventManager(context, "runtime.onStartup", fire => {
> + let listener = () => {
> + if (!context.incognito) {
No need to even add the listener in this case.
::: toolkit/components/extensions/ext-runtime.js:116
(Diff revision 2)
> },
> + runtime_internal: {
No need for a `runtime_internal` namespace. Also, please add a blank line.
::: toolkit/components/extensions/ext-runtime.js:120
(Diff revision 2)
> },
> },
> + runtime_internal: {
> + onStartup: new SingletonEventManager(context, "runtime_internal.onStartup", fire => {
> + let listener = () => {
> + if (extension.startupReason == "APP_STARTUP") {
Nit: s/==/===/
::: toolkit/components/extensions/schemas/runtime.json:594
(Diff revision 2)
> ]
> }
> ]
> + },
> + {
> + "namespace": "runtime_internal",
This is unnecessary.
::: toolkit/components/extensions/test/xpcshell/test_ext_runtime_onInstalled_and_onStartup.js:48
(Diff revision 2)
> + extension.sendMessage("get-on-installed-details");
> + let details = yield extension.awaitMessage("on-installed-details");
> + if (onInstalledReason) {
> + equal(details.reason, onInstalledReason, "runtime.onInstalled fired with the correct reason");
> + } else {
> + equal(details, onInstalledReason, "runtime.onInstalled should not have fired");
I don't understand this assertion...
::: toolkit/components/extensions/test/xpcshell/test_ext_runtime_onInstalled_and_onStartup.js:53
(Diff revision 2)
> + if (onStartupFired) {
> + ok(fired, "runtime.onStartup should have fired");
> + } else {
> + ok(!fired, "runtime.onStartup should not have fired");
> + }
`equal(fired, onStartupFired, ...)`
::: toolkit/components/extensions/test/xpcshell/test_ext_runtime_onInstalled_and_onStartup.js:100
(Diff revision 2)
> + if (message == "get-on-installed-details") {
> + browser.test.sendMessage("on-installed-details", onInstalledDetails);
> + } else if (message == "did-on-startup-fire") {
s/==/===/
Same for below.
Attachment #8806546 -
Flags: review?(kmaglione+bmo)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 8•8 years ago
|
||
mozreview-review-reply |
Comment on attachment 8806546 [details]
Bug 1247435 - Add support for runtime.onStartup
https://reviewboard.mozilla.org/r/89940/#review89618
> setContentWindow should be responsible for setting incognito context.incognito, see 1309610
>
> If all suggested steps in that bug are followed, then you don't need runtime_internal method (because context.incognito is then set both in the child and parent).
>
> Do you think that you can submit a separate patch for that bug (if Kris doesn't have time to fix it)?
Kris suggested I fix context.incognito as part of this bug and I just uploaded a new patch which I think fixes it. Could we maybe repurpose bug 1309610 to just handle adding tests for the context.incognito property?
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 13•8 years ago
|
||
mozreview-review |
Comment on attachment 8807547 [details]
Bug 1247435 - Fix context.incognito (this completes part of bug 1309610)
https://reviewboard.mozilla.org/r/90694/#review90636
Attachment #8807547 -
Flags: review?(kmaglione+bmo) → review+
Comment 14•8 years ago
|
||
mozreview-review |
Comment on attachment 8806546 [details]
Bug 1247435 - Add support for runtime.onStartup
https://reviewboard.mozilla.org/r/89940/#review90638
::: toolkit/components/extensions/ext-runtime.js:26
(Diff revision 5)
> let {extension} = context;
> return {
> runtime: {
> - onStartup: ignoreEvent(context, "runtime.onStartup"),
> + onStartup: new SingletonEventManager(context, "runtime.onStartup", fire => {
> + let listener = () => {
> + if (extension.startupReason === "APP_STARTUP" && !context.incognito) {
Just check `context.incognito` before adding the "startup" listener. There's no point in adding it when we know it won't be needed.
Attachment #8806546 -
Flags: review?(kmaglione+bmo) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 17•8 years ago
|
||
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/7ac1ed5691cf
Fix context.incognito (this completes part of bug 1309610) r=kmag
https://hg.mozilla.org/integration/autoland/rev/c5d86a74509d
Add support for runtime.onStartup r=kmag
Keywords: checkin-needed
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 20•8 years ago
|
||
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/f0c05b86175b
Add support for runtime.onStartup: Add it to API list. r=bustage-fix on a CLOSED TREE
Comment 21•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7ac1ed5691cf
https://hg.mozilla.org/mozilla-central/rev/c5d86a74509d
https://hg.mozilla.org/mozilla-central/rev/f0c05b86175b
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Comment 22•8 years ago
|
||
I was able to reproduce the initial issue on Firefox 51.0a2 (2016-11-08) under Windows 10.
Verified fixed on Firefox 52.0a1 (2016-11-08) under Windows 10 64-bit and Ubuntu 16.04 32-bit.
Tested using several webextensions and “TypeError: chrome.runtime.onStartup is undefined” is no longer thrown in Browser Console.
Status: RESOLVED → VERIFIED
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•