Skip to content
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

aboutNightly component does not implement nsIObserver #57

Closed
xabolcs opened this issue Apr 4, 2012 · 10 comments
Closed

aboutNightly component does not implement nsIObserver #57

xabolcs opened this issue Apr 4, 2012 · 10 comments
Milestone

Comments

@xabolcs
Copy link
Collaborator

xabolcs commented Apr 4, 2012

Error Console says:

While creating services from category 'profile-after-change', service for entry 'AboutNightly', contract ID '@mozilla.org/network/protocol/about;1?what=nightly' does not implement nsIObserver.

By the way, I'm not sure about the observing to profile-after-change is needed.

@whimboo
Copy link
Contributor

whimboo commented Apr 4, 2012

Is this with Firefox or another application?

@xabolcs
Copy link
Collaborator Author

xabolcs commented Apr 4, 2012

At least with Firefox and Thunderbird.

Please note that the level for this message is message (and not error or warning), and about:nightly is fully functional!

Mentioning @k0s, the author of a7dea6a.

@whimboo
Copy link
Contributor

whimboo commented May 2, 2012

Moving to 3.4 because it's not that necessary to get out to our user base as soon as possible.

@xabolcs
Copy link
Collaborator Author

xabolcs commented May 31, 2012

Not sure about Your decision how to resolve this, therefore I request two pull, excluding each other.
One for implementing nsIObserver, another one for dropping the profile-after-change registration.

Feel free to close the not merged one!

xabolcs added a commit to xabolcs/nightlytt that referenced this issue May 31, 2012
xabolcs added a commit to xabolcs/nightlytt that referenced this issue May 31, 2012
@whimboo
Copy link
Contributor

whimboo commented Jun 3, 2012

So we should first check why we have this category listed in the manifest and have to listen for the profile-after-change observer. Please try to investigate which code gets executed at this point by adding dump() calls to the appropriate nightly component and start Firefox from the console. What is 'category' in manifest.rdf declaring in detail? I'm not familiar with that code right now.

@xabolcs
Copy link
Collaborator Author

xabolcs commented Jun 5, 2012

whimboo commented

So we should first check why we have this category listed in the manifest and have to listen for the profile-after-change observer.

IMO the listing is just a mistake. Check the MDN page about custom about: URIs!
And think a little bit! :)
profile-after-change is a startup notificaton in place of app-startup and xpcom-startup.
There is nothing to do on that point for about:nightly. It should do something only if it gets opened. :)
After implementing some enhancement (#58, #67) too! Only if it's opened!

IMO there is no need to do thos investigating:

Please try to investigate which code gets executed at this point by adding dump() calls to the appropriate nightly component and start Firefox from the console.

whimboo commented

What is 'category' in manifest.rdf declaring in detail? I'm not familiar with that code right now.

Read the appropriate section of XPCOM changes at MDN!
In summary: If your extension currently observes either xpcom-startup or app-startup, you need to update your code to observe profile-after-change instead.

In case of about:nightly there is no need to observing the startup!

EDIT at 2012-07-13

But, for example while improving nttAddonCompatibilityService.js (#52) I would do some stuff when the application starts up.

@whimboo
Copy link
Contributor

whimboo commented Jul 2, 2012

So please come up with a pull request based on your commit:
xabolcs@7a1bb02

@xabolcs
Copy link
Collaborator Author

xabolcs commented Jul 13, 2012

whimboo commented

So please come up with a pull request based on your commit:
xabolcs@7a1bb02

I don't see any reason to came up such a pull request (anyway, there is a pull request already: #78)!
What should do an about:something page at Application startup?
By the way, not registering for notification is at least 0.0000001% increase of Application startup. :D

So I vote to for pull #77!

Also You could see in 95d4f08 (pull #82) I managed to improve nttAddonCompatibilityService.js
without the need of observing Application startup.

@whimboo
Copy link
Contributor

whimboo commented Aug 7, 2012

Ok, thanks for this feedback and the explanation. It makes sense to not register for the observer. I have also tested this change in #77 and about:nightly still works. So lets take #77 then!

@whimboo
Copy link
Contributor

whimboo commented Aug 7, 2012

Pull #77 has been landed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants