Closed
Bug 1243779
Opened 8 years ago
Closed 8 years ago
Remove uriIsPrefix option from nsINavHistoryQuery
Categories
(Toolkit :: Places, defect, P2)
Toolkit
Places
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 | ||
Updated•8 years ago
|
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•8 years ago
|
||
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.
Assignee | ||
Comment 2•8 years ago
|
||
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.
Assignee | ||
Comment 3•8 years ago
|
||
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
Assignee | ||
Comment 4•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/34515/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/34515/
Attachment #8718327 -
Flags: review?(adw)
Comment 5•8 years ago
|
||
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+
Comment 7•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/cf16002d74e4
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in
before you can comment on or make changes to this bug.
Description
•