Closed Bug 1243779 Opened 4 years ago Closed 4 years ago

Remove uriIsPrefix option from nsINavHistoryQuery

Categories

(Toolkit :: Places, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: mak, Assigned: mak)

References

Details

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

Attachments

(1 file)

We cannot support this option after bug 889561.

The most problematic part is figuring how many add-ons may be relying on it since the add-ons sdk is exposing it.
Assignee: nobody → mak77
Status: NEW → ASSIGNED
I went through the addons on mxr, most of them are just including the addons-sdk but not using the option, many others are using another library that has a bookmarks.js library that uses it internally, but they don't seem to make use of it.
A couple add-ons are setting it, even if they are not searching for a uri prefix, they will continue working once we remove it (it will just set an unused expando on the xpcom wrapper). I suspect most of the usage in the wild is exactly setting it wrongly to instead match a specific uri, that will continue working.

In the end I found just one add-on, in preliminary review state, using this in the expected way: https://addons.mozilla.org/en-US/firefox/addon/safer-route/
Looks like it's using the prefix to check if a page is https. I think it's not exactly the kind of usage it was designed for, but may be a valid use-case. We don't have an API for that, on the other hand it should be possible, even in the "new world" without uriIsPrefix, to run a query using the new hash column we will add, since I plan to retain a readable schema in the hash for non http urls. Or it could run a custom query first limiting on domain (rev_host), then on the prefix, like we are doing in bug 1243778.
We could add a new "schemaIs" option to nsNavHistoryQuery, but that API is synchronous, soon or later should be rewritten, don't think it's the right thing to spend time on now. If we'll ever have a compelling need in the product we can add an hostHasScheme helper in PlacesUtils.
I'll see how hard is to add a schema filter to the query API in bug 889561, it may be trivial in the end, we'll see. The scope is to cover the most common cases where one wants to find pages by schema and host.
I found another couple add-ons using the add-ons sdk search capability. In the end I decided to keep supporting the functionality in the add-ons sdk, I query by hostname and then filter the results later. This may also be a good solution for most add-ons in need of this functionality, or they can just use the sdk.
Keywords: addon-compat
Comment on attachment 8718327 [details]
MozReview Request: Bug 1243779 - Remove uriIsPrefix option from nsINavHistoryQuery.r=adw

https://reviewboard.mozilla.org/r/34515/#review31473
Attachment #8718327 - Flags: review?(adw) → review+
https://hg.mozilla.org/mozilla-central/rev/cf16002d74e4
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.