Show compatibility error if an add-on uses onProxyAvailable or asyncResolve

RESOLVED FIXED in 2015-04

Status

addons.mozilla.org Graveyard
Compatibility Tools
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: jorgev, Assigned: mstriemer)

Tracking

unspecified
2015-04

Details

(Whiteboard: [fx38])

(Reporter)

Description

3 years ago
As explained in bugs 436344 and 1125372, some changes were performed to proxy networking interfaces. All instances of the onProxyAvailable and asyncResolve functions need to be flagged.

This is a compatibility error, so the add-on shouldn't be upgraded in this case. It should also appear as a warning in regular validations.

Message:

The onProxyAvailable and asyncResolve functions have changed. They now take an nsIChannel instead of an nsIURI as an argument. See <LINK> for more information.

Link: https://bugzilla.mozilla.org/show_bug.cgi?id=436344

Affected add-on:

https://addons.mozilla.org/addon/chatzilla/

Release timing:

Firefox 38 will be released on May 12th.
(Assignee)

Updated

3 years ago
Blocks: 1153874
(Assignee)

Comment 2

3 years ago
https://github.com/mozilla/amo-validator/commit/9fa57a45d56f60eccccf8c82401888df64eb97d7
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED

Comment 3

3 years ago
I'm going to reopen this rather than file a new bug. This warning is wrong.

This was added for bug 436344, but rendered obsolete by bug 1125372. Instead of "nsIChannel instead of an nsIURI" it is now "nsIChannel or an nsIURI". It's no longer a compatibility breaking change as both capabilities are supported. Both bugs landed in 38.

There's an argument to be made that the warning could be changed to a message indicating that API users that have an nsIChannel should just pass it in instead of having to create an nsIURI, but if that's not the case then no action is needed and this can just be reverted. Not sure which is preferred. In either case, the current warning is providing wrong information.
Status: RESOLVED → REOPENED
Flags: needinfo?(mstriemer)
Resolution: FIXED → ---
(Assignee)

Comment 4

3 years ago
I thought that might have been the case but I left this test in since there were reports of there still being issues after bug 1125372 landed (in the same bug 1125372 comment 22 and bug 1125372 comment 22).

Both of those reports are about FoxyProxy so it could be something specific to them but it seems like maybe <38 and >=38 aren't quite the same?

If there aren't any discrepancies then I'd say we remove the test as it isn't very helpful in that case.
Flags: needinfo?(mstriemer) → needinfo?(davemgarrett)

Comment 5

3 years ago
There were FoxyProxy specific issues, in fact, they apparently were one of those lobbying for the change in the first place: bug 1125372 comment 10. People been changing around the core proxy APIs for a while now, and I don't remember which other changes were done when. (my involvement is tangential, and as noted in that bug, I just have a "works well enough" check to make sure I don't leak DNS requests in Flagfox by accident when that's supposed to be disabled)

If there is still a discrepancy here, I don't know what it is, but the warning as-is doesn't tell you either so that's not much help. I doubt there's enough developers that would really need any message about this API change if you were to downgrade it from a warning, so just removing it sounds like the simplest route.
Flags: needinfo?(davemgarrett)

Comment 6

3 years ago
Note, by the way, that bug 1125372 was filed by a ChatZilla developer, which is the affected addon that was cited in comment 0 above as a reason for filing this bug. They haven't needed to put out a new update to maintain compatibility. (I just tested it in Firefox 38, to be sure; connected to #Firefox fine) If there's another issue with the proxy API, it's probably another issue entirely.

Comment 7

2 years ago
Based on the last few comments, it looks like there isn't much to do here and with Firefox 38 being a few versions ago, closing. If anyone feels this should be re-opened, please do so.
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago2 years ago
Resolution: --- → FIXED
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.