-
Notifications
You must be signed in to change notification settings - Fork 56
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
Support onEnabled event when the extension from disabled state to enabled state #353
Comments
I'm supportive of supporting |
1 and 3 can work, 2 definitely won't because runtime.onInstalled clearly and unequivocally says it's about installation but none happens on re-enabling. |
From the practical point of view, I would like this to be a Since extensions are updated even when they are disabled, they are missing all |
Having a misleading API is not really practical, so onInstalled should be about installations.
That's a good point. I guess the solution might be to dispatch the missing onInstalled either in addition to onEnabled or instead of it. In both cases the onInstalled event parameter should probably contain an additional property like |
An edge case is that an extension is updated to a new version, then the browser find it has new warning permissions and disable it. The user may manually enable it later (for a short or long time). When it is enabled again, there should be only an "enable" event and no "update" event. In the "enable" event, developers usually execute some one time startup logic code and should not reply on the "previousVersion" (developers can check their own flags). So the "enable" event also means it may be updated. |
Quoting @oliverdunk in https://github.com/w3c/webextensions/blob/main/_minutes/2023-02-16-wecg.md
This is not true. The event is actually fired for an unpacked extension in Chrome when you reload it because reloading is the same as an update, which includes re-installation. Naturally onInstalled won't be fired when the extension was disabled and then re-enabled without reinstalling because there was no installation and logically this should remain unchanged i.e. I disagree with Rob's idea because it's a violation of the good programming practice that a function does the thing implied by its name and doesn't do things outside the scope implied by its name. |
Sorry, I think the minutes slightly missed what I was trying to say (which is totally fine, taking minutes is hard!). All I wanted to point out is that there are some interesting non-intuitive behaviours for unpacked extensions. One that comes to mind is that I don't think you can easily test the update reason. |
You can simply click the reload icon. What's not trivial is to emulate automatic update behavior but that's beside the point here. |
@tophf: I just tested and you're totally right. I'm not sure what behaviour I was thinking of but clearly it wasn't this. On the larger proposal, I've followed up about this on the Chrome side. We're generally supportive of the idea, but don't feel super comfortable with using the existing event. We would prefer something that we can be sure won't break extensions making a false assumption based on the function name (admittedly this is already a problem with install vs. update for example, but it would be nice to avoid making it worse). It might be worth having some more discussion here either in the next meeting or just when we next decide there's a browser that's looking to implement it. |
I proposed three solutions in my original post, all of which make sense to some extent. At present, there is no workaround for onEnabled event. So either solution is better than no solution. About name problem, If the name is the last stumbling block for this feature, perhaps the option one is more appropriate. |
onStartup at least does not lie about it being a |
Tweaking the title as this is more of a feature request than a specific proposal. |
This is already a simple and clear proposal. The functionality is clear and the implementation is not a problem. If browser vendors can reach an agreement, it can be done quickly. |
Sharing some thoughts ahead of the meeting. From what I can tell there are two main use cases (I'm sure there are more, so please feel free to call those out):
With those in mind, I wonder if something like a new runtime.onExtensionStartup would be most helpful, which fires on both profile startup and when the extension is enabled. Perhaps it could optionally have some sort of The reason I'm wondering about this instead of onEnabled is that it solves both use cases whereas onEnabled only solves the first. Also another thought - I could see an argument that not firing runtime.onInstalled for updates after an extension is enabled is a bug. Maybe we just fix that and we solve the first use case without anything new at all? This is basically the discussion in https://bugs.chromium.org/p/chromium/issues/detail?id=388231 which I realise hasn't moved for a while. Thinking out loud here, haven't spoken to the rest of the Chrome team about this... |
What is the definition of an extension "startup"? It's a bit confusing. Many people intuitively think that "startup" is the first execution in an extension life cycle. By this definition, "extension startup" should include:
The above cases fully cover the first execution in an extension life cycle. For multiple profiles browser like Chrome, assuming two profile windows are opened, will extensions be terminated when one profile window is closed? In my test, the answer is "No". A persistent MV2 extension is still alive when the profile window is closed. Alarm events(MV2 and MV3) still fired when the profile window is closed. So only when the profile first startup means extensions in that profile first startup. Therefore, browser profile startup does not necessarily mean extension startup. |
@hanguokai I think all of those should count 🙂
Do you think there are cases where this distinction is important for the API to be useful? I'm leaning towards no, but we should still do our best to define it so it can be consistent between browsers. |
@oliverdunk According to the definition of extension startup ( If you want to introduce |
@hanguokai / anyone else: I've been trying to reproduce the issue with onInstalled not firing for disabled extensions, but I haven't been able to reproduce it [1]. Are you hitting this, and do you have steps to reproduce, or is it possible that issue is fixed? I'm not proposing to close this discussion but want to understand what works today. [1] https://bugs.chromium.org/p/chromium/issues/detail?id=388231#c13 |
@oliverdunk Off topic, I suggest that browsers provide a mock method for developers to test upgrades from the extension store. This will solve a lot of debugging and testing problems, including the one you're facing. I'd love to help you, but I don't have an extension at hand that needs to be published. |
On the Chrome side (as I think you've seen) we recently released a testing tool which can be used for this: https://github.com/GoogleChromeLabs/extension-update-testing-tool From the last meeting's minutes:
I mentioned in the last meeting that I'm still optimistic about a runtime.onExtensionStartup event but wanted to put together a more complete list of use cases. I still need to have more conversations on the Chrome side but I have put together a list:
I feel like this shows the wide range of use cases where registering for runtime.onInstalled, runtime.onStartup and runtime.onEnabled would be useful and why having a single event could be beneficial. If anyone can think of anything I've missed, I'd be curious to know. |
Doing some one-time work at application startup (rather than checking the status every time later) is a very common programming pattern, regardless of programming language or programming environment. So, I think that's the obvious thing, and there's no need to add more use cases to prove it's useful. |
While I agree it would be useful, I think there's still general uncertainty about if we add a generic event or just runtime.onEnabled, so I think we still have more people to convince :) |
I've given a lot of thought to all the proposals here. To recap, two things have been proposed.
What I like about the first proposal is that it's a natural pattern for developers who know about The second case is very interesting because, as Oliver pointed out, the vast majority of chrome extensions that want to subscribe to Another issue raised here that seems important to take into account in the final implementation decision is that an extension won't receive the I'm in favor of the generic event, as it can be error-prone to have to subscribe to the three events mentioned above. This will prevent the developers from forgetting one of the extension's states by proposing a higher-level event. We can incorporate an object containing the PS: By the way, I must have missed it in the Chrome documentation, but is |
ExtensionStartup(D) = onInstalled(A) + onStartup(B) + onEnabled(C) At present, A and B already exist, while C and D do not exist yet. If we're not sure whether we should implement C or D , can we implement both? Is there any reason to implement only one of them? |
Yes indeed, I forgot to mention it in my previous comment but I also wondered about it. |
On the Chrome side, we've had some more discussions and are supportive of implementing both events. We're tentatively thinking that runtime.onExtensionLoaded would be a better name for runtime.onExtensionStartup. That's likely a good discussion point for the next meeting. |
Good news, thanks for pushing this forward. |
While implementing C and D, can we please make guarantees about their listeners being called before any other listeners run? People seem to incorrectly assume that's the case. See #464 |
Could you share a more specific behaviour that you're looking for? I would agree with the thoughts in this comment that it seems like things are mostly working as intended. Edit: Thinking about the other issue some more, making sure C and D always fire after onInstalled is an interesting idea. I'd need to check with the Chrome team on that, in particular to see if there are other ways to handle this, but it sounds sensible at first glance. |
A new name Regardless of the name, it might be worth reusing onInstalled's parameter so that the event supersedes the old onStartup/onInstalled with |
Yes, we have started looking at what it would take to implement this :)
Definitely supportive of having some reasons, we have something similar to this in our Chrome API proposal (which we do for our own processes but doesn't replace conversation in the WECG). Hopefully we can make that public soon. |
References:
Problem Description
Extensions that are disabled are very common. You can see what percentage of users have your extension enabled and disabled from the extension store dashboard.
At present, there are some events when an extension startup, i.e.
runtime.onInstalled
andruntime.onStartup
. But there is no event when an extension from disabled state to enabled state, for example, users disable then re-enable an extension.Similar to the purpose of using
onInstalled
andonStartup
event, sometimesonEnabled
event is also needed.Possible Solutions
1. New API:
runtime.onEnabled
2. Fire an event in
runtime.onInstalled
Here, add a new reason in
runtime.OnInstalledReason
.3. Allow to use
management.onEnabled
without the "management" permissionAt present,
management.onEnabled
is required "management" permission to use it, even though you just listen yourself.The text was updated successfully, but these errors were encountered: