Closed Bug 1145860 Opened 9 years ago Closed 8 years ago

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

Categories

(addons.mozilla.org Graveyard :: Compatibility Tools, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
2015-04

People

(Reporter: jorgev, Assigned: mstriemer)

References

Details

(Whiteboard: [fx38])

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.
Blocks: 1153874
https://github.com/mozilla/amo-validator/commit/9fa57a45d56f60eccccf8c82401888df64eb97d7
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
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 → ---
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)
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)
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.
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
Closed: 9 years ago8 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.