Closed Bug 907893 Opened 6 years ago Closed 6 years ago

nsIBrowserSearchEngine interface on FF24b needs a uuid bump

Categories

(Firefox :: Search, defect)

24 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 26
Tracking Status
firefox24 + fixed
firefox25 + fixed
firefox26 --- fixed

People

(Reporter: WeirdAl, Assigned: Gavin)

References

()

Details

(Keywords: addon-compat, Whiteboard: [qa-])

Attachments

(1 file)

On FF24b, there was a change to nsIBrowserSearchService's methods (addEngine in particular) that didn't get a corresponding uuid change.  The latest uuid change to that interface appears to predate the method change.

I haven't checked FF25 aurora yet, or trunk.
Blocks: 493051
Attached patch uuid bumpSplinter Review
[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 493051
User impact if declined: potential for crashing binary addons
Testing completed (on m-c, etc.): N/A
Risk to taking this patch (and alternatives if risky): compatibility break for addons on beta that have already been compiled. Alternative for beta is to not take the change, in which case addons calling this method may crash.
String or IDL/UUID changes made by this patch: yes, that's the intent.
Attachment #794266 - Flags: approval-mozilla-beta?
Attachment #794266 - Flags: approval-mozilla-aurora?
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #1)
> Created attachment 794266 [details] [diff] [review]
> uuid bump
> 
> [Approval Request Comment]
> Bug caused by (feature/regressing bug #): bug 493051
> User impact if declined: potential for crashing binary addons
> Testing completed (on m-c, etc.): N/A
> Risk to taking this patch (and alternatives if risky): compatibility break
> for addons on beta that have already been compiled. Alternative for beta is
> to not take the change, in which case addons calling this method may crash.
> String or IDL/UUID changes made by this patch: yes, that's the intent.

Do we know of any addon's/plugins that using this interface and may be impacted ? In the past we have gone with option of actually not bumping the IID post Beta 1 and since the addon testing is already completed by this time and instead did some outreach letting them know about the issue.But, :Gavin points out the risk of a potential crash in this situation :( Is taking the patch the lowest risk option here ?
The potential breakage here would happen with binary add-ons, and we never know for sure which interfaces they are using, since they are mostly hosted outside of AMO. Given that many alter search settings it's very possible that some would be affected...

I don't think there's a good solution here, but I would favor doing the uuid bump for the sake of correctness.
(In reply to Jorge Villalobos [:jorgev] from comment #3)
> The potential breakage here would happen with binary add-ons, and we never
> know for sure which interfaces they are using, since they are mostly hosted
> outside of AMO. Given that many alter search settings it's very possible
> that some would be affected...
> 
> I don't think there's a good solution here, but I would favor doing the uuid
> bump for the sake of correctness.

Thanks Jorgev, Approving the patch and I will work with you on proactively trying to contact popular addon's like we've done in the past.
Attachment #794266 - Flags: approval-mozilla-beta?
Attachment #794266 - Flags: approval-mozilla-beta+
Attachment #794266 - Flags: approval-mozilla-aurora?
Attachment #794266 - Flags: approval-mozilla-aurora+
Keywords: checkin-needed
Whiteboard: [c-n for beta/aurora]
Apparently landing this required a clobber on Windows.
https://hg.mozilla.org/mozilla-central/rev/6159d3584b37
Status: NEW → RESOLVED
Closed: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
Assuming no QA needed here. Naturally we'll keep an eye out for any addon-compat issues as is our normal process. Please remove [qa-] from the whiteboard and add the verifyme keyword if this needs QA.
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.