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

Update Add-on Compatibility prefs #48

Merged
merged 0 commits into from
Mar 26, 2012
Merged

Conversation

brianking
Copy link
Contributor

For issue #14. The latest Aurora is 13, but I have added a couple of versions further to cover us for a couple more releases.

.xxb prefs are no longer needed as we don't use beta versioning in Firefox anymore.

@brianking
Copy link
Contributor Author

Hrm, I am wondering why a new issue was created for this when I already linked the pull request to Issue #14?

@whimboo
Copy link
Contributor

whimboo commented Mar 22, 2012

It's strange, right. And I haven't figured out how to prevent this yet. So far you can only circumvent this by using the RestAPI when submitting a pull request.

"extensions.checkCompatibility.2.10",
"extensions.checkCompatibility.2.10a",
"extensions.checkCompatibility.2.11",
"extensions.checkCompatibility.2.11a",
"extensions.checkCompatibility.nightly"];
}
else if (appInfo.name == "Songbird") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also do the same for Songbird? The current nightlies are at 1.12.0a.

@brianking
Copy link
Contributor Author

This is ready again for review ... 2 commits.


.popup-notification-icon[popupid="nightly-compatibility-restart"] {
list-style-image: url("chrome://nightly/content/brand/logo.png");
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be attached as a separate pull request. Please remove it from that one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do I do that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert this change in your current branch, create a new one and apply it there.

@whimboo
Copy link
Contributor

whimboo commented Mar 26, 2012

Brian, just came into my mind. Haven't we had some magic for ACR which has been detected the application version automatically and setup the right compatibility pref? That way we wouldn't have to do any of the above prefs addition anymore.

@brianking
Copy link
Contributor Author

We ended up not doing that for ACR because we didn't need to in the end when CBD landed. Yes, it would be ideal here but I suggest we land this, file a separate issue, and do it for the release after next.

@whimboo
Copy link
Contributor

whimboo commented Mar 26, 2012

Thanks for the quick response. So yeah, lets hard-code for now and file a new issue. If time permits I'm even happy to fold it into the 3.3 release then.

@xabolcs
Copy link
Collaborator

xabolcs commented Mar 26, 2012

whimboo comented

... which has been detected the application version automatically and setup the right compatibility pref? That way we wouldn't have to do any of the above prefs addition anymore.

That's what I was talking about a month ago. Nobody cares? :(

See my comment in #14:

IMHO if somebody would like to touch nttAddonCompatibilityService.js (read: revamp NTT's force-compatiblity feature) then he/she should consider about Kris Maglione's work in his addon in addition - how to set only the necessary and sufficient subset of extensions.checkCompatibility.* preferences.

@brianking brianking merged commit 997df8e into mozilla:master Mar 26, 2012
@whimboo
Copy link
Contributor

whimboo commented Mar 26, 2012

@xabolcs we simply forgot about that. Can you please file the new issue and c&p that content in there? Thanks.

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

Successfully merging this pull request may close these issues.

3 participants