Closed
Bug 702639
Opened 13 years ago
Closed 13 years ago
Kill excludeItemsIfParentHasAnnotation query option
Categories
(Toolkit :: Places, defect)
Toolkit
Places
Tracking
()
RESOLVED
FIXED
mozilla13
People
(Reporter: mak, Assigned: mak)
References
(Blocks 1 open bug)
Details
(Keywords: addon-compat, dev-doc-complete, perf)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Gavin
:
superreview+
|
Details | Diff | Splinter Review |
This existed mostly for livemarks, it's complex and should just die for now.
Assignee | ||
Comment 1•13 years ago
|
||
I think I may be able to remove another rock from FilterResultSet, and that would be another step towards async queries, let's see what I'll end up with.
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•13 years ago
|
||
So, the last vestiges of FilterResultSet() exist just to remove duplicate uris from tags and check if a node can be added to a query with search terms.
The former will be fixable changing tags schema, the latter is not a bug, once the former is fixed FilterResultSet can be merged into EvaluateQueryForNode (or become an helper like EvaluateSearchTermsForNode).
I wonder if I could subquery tags, so that the external query removes duplicates and completely kill FilterResultSet() now, would be fancy.
Assignee | ||
Comment 3•13 years ago
|
||
Comment on attachment 578053 [details] [diff] [review]
patch v1.0
note that this depends on the livemarks changes, so it can't land before.
Attachment #578053 -
Flags: review?(dietrich)
Assignee | ||
Comment 4•13 years ago
|
||
Comment on attachment 578053 [details] [diff] [review]
patch v1.0
Review of attachment 578053 [details] [diff] [review]:
-----------------------------------------------------------------
::: toolkit/components/places/nsNavHistory.cpp
@@ +4137,5 @@
> .Str(", h.url, page_title, tags, ")
> .Str(nsPrintfCString(17, "0, 0, 0, 0, %d, 0)",
> mozIPlacesAutoComplete::MATCH_ANYWHERE_UNMODIFIED).get());
> + // Serching by terms implicitly exclude queries.
> + clause.Condition("NOT url BETWEEN 'place:' AND 'place;'");
self-comment: should be h.url
Comment 5•13 years ago
|
||
Comment on attachment 578053 [details] [diff] [review]
patch v1.0
Review of attachment 578053 [details] [diff] [review]:
-----------------------------------------------------------------
looks ok, r=me
Attachment #578053 -
Flags: review?(dietrich) → review+
Assignee | ||
Updated•13 years ago
|
Blocks: placesFolders
Assignee | ||
Comment 6•13 years ago
|
||
fixes a subtle bug with excludeQueries that was failing a test on Try.
Attachment #578053 -
Attachment is obsolete: true
Assignee | ||
Updated•13 years ago
|
Whiteboard: [needs SR]
Assignee | ||
Comment 7•13 years ago
|
||
Comment on attachment 596000 [details] [diff] [review]
patch v1.1
Review of attachment 596000 [details] [diff] [review]:
-----------------------------------------------------------------
oops, I forgot I still need a SR here :)
This options was useful only to filter out livemark children from queries, now that load-on-demand livemarks exist it is no more useful.
The reason we can't just deprecate it is that its removal allows to speed up quite some queries, on the other side the places query strings just ignore unknown options, and this will just become one of those, so compatibility issues should be limited.
Attachment #596000 -
Flags: superreview?(gavin.sharp)
Updated•13 years ago
|
Attachment #596000 -
Flags: superreview?(gavin.sharp) → superreview+
Assignee | ||
Comment 8•13 years ago
|
||
Whiteboard: [needs SR]
Target Milestone: --- → mozilla13
Assignee | ||
Comment 9•13 years ago
|
||
I don't expect any interesting breakage from this, btw adding the keyword.
Keywords: addon-compat
Assignee | ||
Comment 10•13 years ago
|
||
and documentation may have some pointer to this
Keywords: dev-doc-needed
Assignee | ||
Comment 11•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 12•13 years ago
|
||
Docs updated:
https://developer.mozilla.org/en/Places_query_URIs#Query_operators
https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsINavHistoryQueryOptions#Attributes
Mentioned on Firefox 13 for developers.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•