nsIUrlListManager.safeLookup not updated with bug 775796

RESOLVED FIXED in Firefox 19

Status

()

Toolkit
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: hectorz, Assigned: mounir)

Tracking

({addon-compat, dev-doc-needed})

Trunk
mozilla20
addon-compat, dev-doc-needed
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox17 affected, firefox18- affected, firefox19- fixed, firefox20 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
nsIUrlListManager.safeLookup is said to be the exception free wrapper[1] for nsIUrlClassifierDBService.lookup, but it was not changed with bug 775796 where the first argument to lookup switched from string to principal, and stops working since fx 17.

[1]: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/url-classifier/content/listmanager.js#309
(Reporter)

Updated

5 years ago
Blocks: 775796
(Assignee)

Updated

5 years ago
Assignee: nobody → mounir
Status: NEW → ASSIGNED
OS: Windows 7 → All
Hardware: x86 → All
Version: unspecified → Trunk
(Assignee)

Comment 1

5 years ago
Created attachment 681049 [details] [diff] [review]
Patch
Attachment #681049 - Flags: review?(bzhao)
(Reporter)

Comment 2

5 years ago
Comment on attachment 681049 [details] [diff] [review]
Patch

Review of attachment 681049 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not experienced enough to do a formal review, but with the changes in (https://hg.mozilla.org/mozilla-central/diff/88af948f119e/toolkit/components/url-classifier/nsIUrlClassifierDBService.idl) as a reference, this patch looks good to me.

Maybe a testcase should be included to make sure this doesn't happen again?
Attachment #681049 - Flags: review?(bzhao) → feedback+
(Assignee)

Updated

5 years ago
Attachment #681049 - Flags: superreview?(jonas)
Attachment #681049 - Flags: superreview?(jonas) → superreview+
(Assignee)

Comment 3

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ab84754b536
Target Milestone: --- → mozilla19
https://hg.mozilla.org/mozilla-central/rev/8ab84754b536
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: mozilla19 → mozilla20
Depends on: 813691
Bug 775796 may provide some hints
Opps sorry commented on wrong bug.
(Reporter)

Comment 7

5 years ago
Not really sure, but I remember once being told these flags and keywords should be set.
status-firefox17: --- → affected
status-firefox18: --- → affected
status-firefox19: --- → affected
tracking-firefox18: --- → ?
tracking-firefox19: --- → ?
Keywords: dev-doc-needed
(Assignee)

Comment 8

5 years ago
Comment on attachment 681049 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 775796
User impact if declined: developers will no longer be able to use nsIUrlListManager.safeLookup
Risk to taking this patch (and alternatives if risky): there is only an UUID change, this is the only risk (there is no other code change). We might break addon compat but I believe we are soon enough in the m-a and m-b cycle that we could land this quite safely.
String or UUID changes made by this patch: yes, nsIUrlListManager
Attachment #681049 - Flags: approval-mozilla-beta?
Attachment #681049 - Flags: approval-mozilla-aurora?
(Assignee)

Updated

5 years ago
status-firefox20: --- → fixed
(Assignee)

Comment 9

5 years ago
Note that we might think to land this in ESR (and Firefox 17?) if addons happen to be broken because of this.
(In reply to Mounir Lamouri (:mounir) from comment #8)
> [Approval Request Comment]
> User impact if declined: developers will no longer be able to use
> nsIUrlListManager.safeLookup

This appears to be a regression in FF17. Do we know of any add-ons broken by bug 775796? We need to be able to weigh maintaining the status quo for another 6 weeks versus addon compatibility impact.

> String or UUID changes made by this patch: yes, nsIUrlListManager

Including Jorge to help make the call on addon compatibility impact to beta here.

We'll approve for Aurora in the meantime.
Flags: needinfo?(jorge)
Keywords: addon-compat

Updated

5 years ago
Attachment #681049 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I don't see any add-ons using safeLookup. Changing the interface now sounds okay to me for beta, but there is no urgency to push this through for add-ons.

I do see a couple of add-ons using functions that changed on bug 775796, which wasn't flagged addon-compat :\. However, they are very few and I can contact the developers directly.
Flags: needinfo?(jorge)
(Assignee)

Comment 12

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/271ed0c9bc96
status-firefox19: affected → fixed

Updated

5 years ago
tracking-firefox18: ? → -
tracking-firefox19: ? → -
Comment on attachment 681049 [details] [diff] [review]
Patch

Given Jorge's comment, let's leave this as is for FF18 (same as FF17).
Attachment #681049 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
You need to log in before you can comment on or make changes to this bug.