From 714d1552dcb2c646abbd415e2383774759f129c2 Mon Sep 17 00:00:00 2001 From: "dietrich%mozilla.com" Date: Fri, 11 Apr 2008 16:22:02 +0000 Subject: [PATCH] Bug 419731 - Changing bookmark title in places should be reflected in tag containers (for mak77@supereva.it, r=mano, a=beltzner) git-svn-id: svn://10.0.0.236/trunk@250075 18797224-902f-48f8-a5cc-f745e15eee43 --- .../components/places/content/controller.js | 36 ++++- .../components/places/content/menu.xml | 19 ++- .../components/places/content/toolbar.xml | 4 +- .../components/places/content/tree.xml | 9 +- .../components/places/content/treeView.js | 9 +- .../components/places/content/utils.js | 8 +- .../places/src/nsPlacesTransactionsService.js | 7 +- .../places/public/nsINavHistoryService.idl | 12 ++ .../components/places/src/nsNavBookmarks.cpp | 37 +++-- .../components/places/src/nsNavHistory.cpp | 111 +++++++++++++-- .../places/src/nsNavHistoryQuery.cpp | 2 +- .../places/src/nsNavHistoryResult.cpp | 82 ++++++++++- .../places/src/nsNavHistoryResult.h | 1 + .../toolkit/components/places/src/utils.js | 25 +++- .../places/tests/unit/test_419731.js | 128 ++++++++++++++++++ 15 files changed, 435 insertions(+), 55 deletions(-) create mode 100644 mozilla/toolkit/components/places/tests/unit/test_419731.js diff --git a/mozilla/browser/components/places/content/controller.js b/mozilla/browser/components/places/content/controller.js index 95f3847882f..00df7936d51 100755 --- a/mozilla/browser/components/places/content/controller.js +++ b/mozilla/browser/components/places/content/controller.js @@ -75,15 +75,18 @@ const REMOVE_PAGES_MAX_SINGLEREMOVES = 10; * insertion point to accommodate the orientation should be done by * the person who constructs the IP, not the user. The orientation * is provided for informational purposes only! + * @param [optional] aIsTag + * Indicates if parent container is a tag * @constructor */ -function InsertionPoint(aItemId, aIndex, aOrientation) { +function InsertionPoint(aItemId, aIndex, aOrientation, aIsTag) { this.itemId = aItemId; this.index = aIndex; this.orientation = aOrientation; + this.isTag = aIsTag; } InsertionPoint.prototype.toString = function IP_toString() { - return "[object InsertionPoint(folder:" + this.itemId + ",index:" + this.index + ",orientation:" + this.orientation + ")]"; + return "[object InsertionPoint(folder:" + this.itemId + ",index:" + this.index + ",orientation:" + this.orientation + ",isTag:" + this.isTag + ")]"; }; /** @@ -308,7 +311,8 @@ PlacesController.prototype = { * Determines whether or not nodes can be inserted relative to the selection. */ _canInsert: function PC__canInsert() { - return this._view.insertionPoint != null; + var ip = this._view.insertionPoint; + return ip != null && ip.isTag != true; }, /** @@ -854,6 +858,18 @@ PlacesController.prototype = { if (PlacesUtils.nodeIsFolder(node)) removedFolders.push(node); + else if (PlacesUtils.nodeIsTagQuery(node.parent)) { + var queries = asQuery(node.parent).getQueries({}); + if (queries.length == 1) { + var folders = queries[0].getFolders({}); + if (folders.length == 1) { + var uri = PlacesUtils._uri(node.uri); + var tagItemId = folders[0]; + transactions.push(PlacesUIUtils.ptm.untagURI(uri, [tagItemId])); + } + } + continue; + } transactions.push(PlacesUIUtils.ptm.removeItem(node.itemId)); } @@ -1337,9 +1353,17 @@ var PlacesControllerDragHelper = { movedCount++; } - transactions.push(PlacesUIUtils.makeTransaction(unwrapped, - flavor.value, insertionPoint.itemId, - index, copy)); + // if dragging over a tag container we should tag the item + if (insertionPoint.isTag) { + var uri = PlacesUtils._uri(unwrapped.uri); + var tagItemId = insertionPoint.itemId; + transactions.push(PlacesUIUtils.ptm.tagURI(uri,[tagItemId])); + } + else { + transactions.push(PlacesUIUtils.makeTransaction(unwrapped, + flavor.value, insertionPoint.itemId, + index, copy)); + } } var txn = PlacesUIUtils.ptm.aggregateTransactions("DropItems", transactions); diff --git a/mozilla/browser/components/places/content/menu.xml b/mozilla/browser/components/places/content/menu.xml index 03ea3dab7ee..90167a07bcf 100755 --- a/mozilla/browser/components/places/content/menu.xml +++ b/mozilla/browser/components/places/content/menu.xml @@ -189,12 +189,15 @@ @@ -274,7 +277,8 @@ var nodeY = xulNode.boxObject.y - popupY; var nodeHeight = xulNode.boxObject.height; if (xulNode.node && - PlacesUtils.nodeIsFolder(xulNode.node) && + (PlacesUtils.nodeIsFolder(xulNode.node) || + PlacesUtils.nodeIsTagQuery(xulNode.node)) && !PlacesUtils.nodeIsReadOnly(xulNode.node)) { // This is a folder. If the mouse is in the top 25% of the // node, drop above the folder. If it's in the middle @@ -893,8 +897,12 @@ // By default, the insertion point is at the top level, at the end. var index = -1; var folderId = 0; - if (PlacesUtils.nodeIsFolder(this._resultNode)) + var isTag = false; + + if (PlacesUtils.nodeIsFolder(this._resultNode)) { folderId = PlacesUtils.getConcreteItemId(this._resultNode); + isTag = PlacesUtils.nodeIsTagQuery(this._resultNode); + } var selectedNode = this.selectedNode; if (selectedNode) { @@ -907,10 +915,11 @@ // If there is another type of node selected, the insertion point // is after that node. folderId = PlacesUtils.getConcreteItemId(selectedNode.parent); - index = PlacesUtils.getIndexOfNode(selectedNode) + index = PlacesUtils.getIndexOfNode(selectedNode); + isTag = PlacesUtils.nodeIsTagQuery(selectedNode.parent); } } - return new InsertionPoint(folderId, index); + return new InsertionPoint(folderId, index, 1, isTag); ]]> diff --git a/mozilla/browser/components/places/content/toolbar.xml b/mozilla/browser/components/places/content/toolbar.xml index a409ba0f7d3..ee3ec8cbf4c 100755 --- a/mozilla/browser/components/places/content/toolbar.xml +++ b/mozilla/browser/components/places/content/toolbar.xml @@ -380,6 +380,7 @@ // By default, the insertion point is at the top level, at the end. var index = -1; var folderId = PlacesUtils.getConcreteItemId(this._result.root); + var isTag = false; var selectedNode = this.selectedNode; if (selectedNode) { @@ -393,9 +394,10 @@ // is after that node. folderId = PlacesUtils.getConcreteItemId(selectedNode.parent); index = PlacesUtils.getIndexOfNode(selectedNode); + isTag = PlacesUtils.nodeIsTagQuery(selectedNode.parent); } } - return new InsertionPoint(folderId, index, 1); + return new InsertionPoint(folderId, index, 1, isTag); ]]> diff --git a/mozilla/browser/components/places/content/tree.xml b/mozilla/browser/components/places/content/tree.xml index 48d1a1a7a76..527f56480ad 100644 --- a/mozilla/browser/components/places/content/tree.xml +++ b/mozilla/browser/components/places/content/tree.xml @@ -491,6 +491,9 @@ @@ -669,7 +673,8 @@ // If this node is part of a readonly container (e.g. a livemark) it // cannot be moved, only copied, so we must change the action used // by the drag session. - if (PlacesUtils.nodeIsReadOnly(parent)) { + if (PlacesUtils.nodeIsReadOnly(parent) || + PlacesUtils.nodeIsTagQuery(parent)) { dragAction.action = Ci.nsIDragService.DRAGDROP_ACTION_COPY; break; } diff --git a/mozilla/browser/components/places/content/treeView.js b/mozilla/browser/components/places/content/treeView.js index aaa3d559076..09791987ad1 100644 --- a/mozilla/browser/components/places/content/treeView.js +++ b/mozilla/browser/components/places/content/treeView.js @@ -998,7 +998,8 @@ PlacesTreeView.prototype = { if (elt.localName == "tree" && elt.view == this && this.selection.isSelected(aRow)) return false; - if (node.parent && PlacesUtils.nodeIsReadOnly(node.parent)) + if (node.parent && PlacesUtils.nodeIsReadOnly(node.parent) && + !PlacesUtils.nodeIsTagQuery(node)) return false; } @@ -1010,6 +1011,9 @@ PlacesTreeView.prototype = { // either add a helper to PlacesUtils or keep it here and add insertionPoint // to the view interface. _disallowInsertion: function PTV__disallowInsertion(aContainer) { + // allow dropping into Tag containers + if (PlacesUtils.nodeIsTagQuery(aContainer)) + return false; // Disallow insertion of items under readonly folders return (!PlacesUtils.nodeIsFolder(aContainer) || PlacesUtils.nodeIsReadOnly(aContainer)); @@ -1056,7 +1060,8 @@ PlacesTreeView.prototype = { return null; return new InsertionPoint(PlacesUtils.getConcreteItemId(container), - index, orientation); + index, orientation, + PlacesUtils.nodeIsTagQuery(container)); }, drop: function PTV_drop(aRow, aOrientation) { diff --git a/mozilla/browser/components/places/content/utils.js b/mozilla/browser/components/places/content/utils.js index 0a073a79376..52974b07b46 100644 --- a/mozilla/browser/components/places/content/utils.js +++ b/mozilla/browser/components/places/content/utils.js @@ -1163,8 +1163,12 @@ var PlacesUIUtils = { // XXX: Downloads // Tags Query - uri = PlacesUtils._uri("place:folder=TAGS"); - itemId = PlacesUtils.bookmarks.insertBookmark(leftPaneRoot, uri, -1, null); + uri = PlacesUtils._uri("place:type=" + + Ci.nsINavHistoryQueryOptions.RESULTS_AS_TAG_QUERY + + "&sort=" + + Ci.nsINavHistoryQueryOptions.SORT_BY_TITLE_ASCENDING); + title = PlacesUtils.bookmarks.getItemTitle(PlacesUtils.tagsFolderId); + itemId = PlacesUtils.bookmarks.insertBookmark(leftPaneRoot, uri, -1, title); PlacesUtils.annotations.setItemAnnotation(itemId, ORGANIZER_QUERY_ANNO, "Tags", 0, EXPIRE_NEVER); self.leftPaneQueries["Tags"] = itemId; diff --git a/mozilla/browser/components/places/src/nsPlacesTransactionsService.js b/mozilla/browser/components/places/src/nsPlacesTransactionsService.js index b4aca282497..73101b4db26 100644 --- a/mozilla/browser/components/places/src/nsPlacesTransactionsService.js +++ b/mozilla/browser/components/places/src/nsPlacesTransactionsService.js @@ -171,7 +171,7 @@ placesTransactionsService.prototype = { return new placesTagURITransaction(aURI, aTags); }, - untagURI: function placesTagURI(aURI, aTags) { + untagURI: function placesUntagURI(aURI, aTags) { return new placesUntagURITransaction(aURI, aTags); }, @@ -919,12 +919,13 @@ placesTagURITransaction.prototype = { __proto__: placesBaseTransaction.prototype, doTransaction: function PTU_doTransaction() { - if (PlacesUtils.getBookmarksForURI(this._uri).length == 0) { + if (PlacesUtils.getMostRecentBookmarkForURI(this._uri) == -1) { // Force an unfiled bookmark first this._unfiledItemId = PlacesUtils.bookmarks .insertBookmark(PlacesUtils.unfiledBookmarksFolderId, - this._uri, -1, + this._uri, + PlacesUtils.bookmarks.DEFAULT_INDEX, PlacesUtils.history.getPageTitle(this._uri)); } PlacesUtils.tagging.tagURI(this._uri, this._tags); diff --git a/mozilla/toolkit/components/places/public/nsINavHistoryService.idl b/mozilla/toolkit/components/places/public/nsINavHistoryService.idl index c3267978983..aaf4c8f9a80 100644 --- a/mozilla/toolkit/components/places/public/nsINavHistoryService.idl +++ b/mozilla/toolkit/components/places/public/nsINavHistoryService.idl @@ -842,6 +842,11 @@ interface nsINavHistoryQuery : nsISupports [retval,array,size_is(count)] out long long folders); readonly attribute unsigned long folderCount; + /** + * For the special result type RESULTS_AS_TAG_CONTENTS we can define only + * one folder that must be a tag folder. This is not recursive so results + * will be returned from the first level of that folder. + */ void setFolders([const,array, size_is(folderCount)] in long long folders, in unsigned long folderCount); @@ -937,6 +942,13 @@ interface nsINavHistoryQueryOptions : nsISupports */ const unsigned short RESULTS_AS_TAG_QUERY = 6; + /** + * This is a container with an URI result type that contains the last + * modified bookmarks for the given tag. + * Tag folder id must be defined in the query. + */ + const unsigned short RESULTS_AS_TAG_CONTENTS = 7; + /** * The sorting mode to be used for this query. * mode is one of SORT_BY_* diff --git a/mozilla/toolkit/components/places/src/nsNavBookmarks.cpp b/mozilla/toolkit/components/places/src/nsNavBookmarks.cpp index af2364e85de..4fc2a831892 100644 --- a/mozilla/toolkit/components/places/src/nsNavBookmarks.cpp +++ b/mozilla/toolkit/components/places/src/nsNavBookmarks.cpp @@ -152,20 +152,21 @@ nsNavBookmarks::Init() // Results are kGetInfoIndex_* // mDBGetChildren: select all children of a given folder, sorted by position - nsCAutoString selectChildren( - NS_LITERAL_CSTRING("SELECT h.id, h.url, COALESCE(a.title, h.title), " + // This is a LEFT OUTER JOIN with moz_places since folders does not have + // a reference into that table. + rv = dbConn->CreateStatement(NS_LITERAL_CSTRING( + "SELECT h.id, h.url, COALESCE(b.title, h.title), " "h.rev_host, h.visit_count, " SQL_STR_FRAGMENT_MAX_VISIT_DATE( "h.id" ) - ", f.url, null, a.id, " - "a.dateAdded, a.lastModified, " - "a.position, a.type, a.fk " - "FROM moz_bookmarks a " - "LEFT JOIN moz_places h ON a.fk = h.id " - "LEFT OUTER JOIN moz_favicons f ON h.favicon_id = f.id " - "WHERE a.parent = ?1 " - " ORDER BY a.position")); - - rv = dbConn->CreateStatement(selectChildren, getter_AddRefs(mDBGetChildren)); + ", f.url, null, b.id, " + "b.dateAdded, b.lastModified, " + "b.position, b.type, b.fk " + "FROM moz_bookmarks b " + "LEFT OUTER JOIN moz_places h ON b.fk = h.id " + "LEFT OUTER JOIN moz_favicons f ON h.favicon_id = f.id " + "WHERE b.parent = ?1 " + "ORDER BY b.position"), + getter_AddRefs(mDBGetChildren)); NS_ENSURE_SUCCESS(rv, rv); // mDBFolderCount: count all of the children of a given folder @@ -345,12 +346,20 @@ nsNavBookmarks::InitTables(mozIStorageConnection* aDBConn) // Making it compound with "type" speeds up type-differentiation // queries, such as expiration and search. rv = aDBConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( - "CREATE INDEX moz_bookmarks_itemindex ON moz_bookmarks (fk,type)")); + "CREATE INDEX moz_bookmarks_itemindex ON moz_bookmarks (fk, type)")); NS_ENSURE_SUCCESS(rv, rv); // The most common operation is to find the children given a parent and position. rv = aDBConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( - "CREATE INDEX moz_bookmarks_parentindex ON moz_bookmarks (parent,position)")); + "CREATE INDEX moz_bookmarks_parentindex " + "ON moz_bookmarks (parent, position)")); + NS_ENSURE_SUCCESS(rv, rv); + + // fast access to lastModified is useful during sync and to get + // last modified bookmark title for tags container's children. + rv = aDBConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + "CREATE INDEX moz_bookmarks_itemlastmodifiedindex " + "ON moz_bookmarks (fk, lastModified)")); NS_ENSURE_SUCCESS(rv, rv); } diff --git a/mozilla/toolkit/components/places/src/nsNavHistory.cpp b/mozilla/toolkit/components/places/src/nsNavHistory.cpp index 31e3b12b810..b01d7b5b2b8 100644 --- a/mozilla/toolkit/components/places/src/nsNavHistory.cpp +++ b/mozilla/toolkit/components/places/src/nsNavHistory.cpp @@ -1363,11 +1363,26 @@ nsNavHistory::MigrateV6Up(mozIStorageConnection* aDBConn) nsresult nsNavHistory::EnsureCurrentSchema(mozIStorageConnection* aDBConn, PRBool* aDidMigrate) { + // We need an index on lastModified to catch quickly last modified bookmark + // title for tag container's children. This will be useful for sync too. + PRBool lastModIndexExists = PR_FALSE; + nsresult rv = aDBConn->IndexExists( + NS_LITERAL_CSTRING("moz_bookmarks_itemlastmodifiedindex"), + &lastModIndexExists); + NS_ENSURE_SUCCESS(rv, rv); + + if (!lastModIndexExists) { + rv = aDBConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + "CREATE INDEX moz_bookmarks_itemlastmodifiedindex " + "ON moz_bookmarks (fk, lastModified)")); + NS_ENSURE_SUCCESS(rv, rv); + } + // We need to do a one-time change of the moz_historyvisits.pageindex // to speed up finding last visit date when joinin with moz_places. // See bug 392399 for more details. PRBool oldIndexExists = PR_FALSE; - nsresult rv = aDBConn->IndexExists( + rv = aDBConn->IndexExists( NS_LITERAL_CSTRING("moz_historyvisits_pageindex"), &oldIndexExists); NS_ENSURE_SUCCESS(rv, rv); @@ -2860,6 +2875,7 @@ PlacesSQLQueryBuilder::Select() switch (mResultType) { case nsINavHistoryQueryOptions::RESULTS_AS_URI: + case nsINavHistoryQueryOptions::RESULTS_AS_TAG_CONTENTS: rv = SelectAsURI(); NS_ENSURE_SUCCESS(rv, rv); break; @@ -2914,14 +2930,43 @@ PlacesSQLQueryBuilder::SelectAsURI() break; case nsINavHistoryQueryOptions::QUERY_TYPE_BOOKMARKS: - mQueryString = NS_LITERAL_CSTRING( - "SELECT b.fk, h.url, COALESCE(b.title, h.title), h.rev_host, " - "h.visit_count," - SQL_STR_FRAGMENT_MAX_VISIT_DATE( "b.fk" ) - ", f.url, null, b.id, b.dateAdded, b.lastModified " - "FROM moz_bookmarks b " - "JOIN moz_places h ON b.fk = h.id AND b.type = 1 " - "LEFT OUTER JOIN moz_favicons f ON h.favicon_id = f.id "); + if (mResultType == nsINavHistoryQueryOptions::RESULTS_AS_TAG_CONTENTS) { + nsNavHistory* history = nsNavHistory::GetHistoryService(); + NS_ENSURE_STATE(history); + + // Order-by clause is hardcoded because we need to discard duplicates + // in FilterResultSet. We will retain only the last modified item, + // so we are ordering by place id and last modified to do a faster + // filtering. + mSkipOrderBy = PR_TRUE; + + mQueryString = NS_LITERAL_CSTRING( + "SELECT b2.fk, h.url, COALESCE(b2.title, h.title), h.rev_host, " + "h.visit_count, " + SQL_STR_FRAGMENT_MAX_VISIT_DATE( "b2.fk" ) + ", f.url, null, b2.id, b2.dateAdded, b2.lastModified " + "FROM moz_bookmarks b2 " + "JOIN moz_places h ON b2.fk = h.id AND b2.type = 1 " + "LEFT OUTER JOIN moz_favicons f ON h.favicon_id = f.id " + "WHERE b2.id IN (" + "SELECT b1.id FROM moz_bookmarks b1 " + "WHERE b1.fk IN " + "(SELECT b.fk FROM moz_bookmarks b WHERE b.type = 1 {ADDITIONAL_CONDITIONS}) " + "AND NOT EXISTS " + "(SELECT id FROM moz_bookmarks WHERE id = b1.parent AND parent = ") + + nsPrintfCString("%d", history->GetTagsFolder()) + + NS_LITERAL_CSTRING(")) ORDER BY b2.fk DESC, b2.lastModified DESC"); + } + else { + mQueryString = NS_LITERAL_CSTRING( + "SELECT b.fk, h.url, COALESCE(b.title, h.title), h.rev_host, " + "h.visit_count," + SQL_STR_FRAGMENT_MAX_VISIT_DATE( "b.fk" ) + ", f.url, null, b.id, b.dateAdded, b.lastModified " + "FROM moz_bookmarks b " + "JOIN moz_places h ON b.fk = h.id AND b.type = 1 " + "LEFT OUTER JOIN moz_favicons f ON h.favicon_id = f.id "); + } break; default: @@ -3142,12 +3187,12 @@ PlacesSQLQueryBuilder::SelectAsTag() mHasDateColumns = PR_TRUE; mQueryString = nsPrintfCString(2048, - "SELECT null, 'place:type=%ld&queryType=%d&excludeQueries=1&folder=' || id, " + "SELECT null, 'place:folder=' || id || '&queryType=%d&type=%ld', " "title, null, null, null, null, null, null, dateAdded, lastModified " "FROM moz_bookmarks " "WHERE parent = %ld", - nsINavHistoryQueryOptions::RESULTS_AS_URI, nsINavHistoryQueryOptions::QUERY_TYPE_BOOKMARKS, + nsINavHistoryQueryOptions::RESULTS_AS_TAG_CONTENTS, history->GetTagsFolder()); return NS_OK; @@ -4919,6 +4964,13 @@ nsNavHistory::QueryToSelectClause(nsNavHistoryQuery* aQuery, // const // all URLs with that annotation } + // parent parameter is used in tag contents queries. + // Only one folder should be defined for them. + if (aOptions->ResultType() == nsINavHistoryQueryOptions::RESULTS_AS_TAG_CONTENTS && + aQuery->Folders().Length() == 1) { + clause.Condition("b.parent =").Param(":parent"); + } + clause.GetClauseString(*aClause); return NS_OK; } @@ -5048,6 +5100,14 @@ nsNavHistory::BindQueryClauseParameters(mozIStorageStatement* statement, NS_ENSURE_SUCCESS(rv, rv); } + // parent parameter + if (aOptions->ResultType() == nsINavHistoryQueryOptions::RESULTS_AS_TAG_CONTENTS && + aQuery->Folders().Length() == 1) { + rv = statement->BindInt64Parameter(index.For(":parent"), + aQuery->Folders()[0]); + NS_ENSURE_SUCCESS(rv, rv); + } + NS_ENSURE_SUCCESS(index.Result(), index.Result()); return NS_OK; @@ -5166,6 +5226,7 @@ nsNavHistory::FilterResultSet(nsNavHistoryQueryResultNode* aQueryNode, ParseSearchTermsFromQueries(aQueries, &terms); PRUint32 queryIndex; + PRUint16 resultType = aOptions->ResultType(); // The includeFolders array for each query is initialized with its // query's folders array. We add sub-folders as we check items. @@ -5235,7 +5296,10 @@ nsNavHistory::FilterResultSet(nsNavHistoryQueryResultNode* aQueryNode, for (queryIndex = 0; queryIndex < aQueries.Count() && !appendNode; queryIndex++) { // parent folder - if (includeFolders[queryIndex]->Length() != 0) { + // RESULTS_AS_TAG_CONTENTS changes bookmarks parent, so we must not filter + // this kind of result based on the parent. + if (includeFolders[queryIndex]->Length() != 0 && + resultType != nsINavHistoryQueryOptions::RESULTS_AS_TAG_CONTENTS) { // filter out simple history nodes from bookmark queries if (aSet[nodeIndex]->mItemId == -1) continue; @@ -5314,6 +5378,14 @@ nsNavHistory::FilterResultSet(nsNavHistoryQueryResultNode* aQueryNode, appendNode = PR_TRUE; } + + // RESULTS_AS_TAG_CONTENTS returns a set ordered by place_id and + // lastModified. So, to remove duplicates, we can retain the first result + // for each uri. + if (resultType == nsINavHistoryQueryOptions::RESULTS_AS_TAG_CONTENTS && + nodeIndex > 0 && aSet[nodeIndex]->mURI == aSet[nodeIndex-1]->mURI) + continue; + if (appendNode) aFiltered->AppendObject(aSet[nodeIndex]); @@ -5514,8 +5586,16 @@ nsNavHistory::RowToResult(mozIStorageValueArray* aRow, aOptions->ResultType() != nsINavHistoryQueryOptions::RESULTS_AS_TAG_QUERY) (*aResult)->GetAsContainer()->mOptions = aOptions; + + // RESULTS_AS_TAG_QUERY has date columns + if (aOptions->ResultType() == nsNavHistoryQueryOptions::RESULTS_AS_TAG_QUERY) { + (*aResult)->mDateAdded = aRow->AsInt64(kGetInfoIndex_ItemDateAdded); + (*aResult)->mLastModified = aRow->AsInt64(kGetInfoIndex_ItemLastModified); + } + return rv; - } else if (aOptions->ResultType() == nsNavHistoryQueryOptions::RESULTS_AS_URI) { + } else if (aOptions->ResultType() == nsNavHistoryQueryOptions::RESULTS_AS_URI || + aOptions->ResultType() == nsNavHistoryQueryOptions::RESULTS_AS_TAG_CONTENTS) { *aResult = new nsNavHistoryResultNode(url, title, accessCount, time, favicon); if (!*aResult) @@ -6026,6 +6106,11 @@ GetSimpleBookmarksQueryFolder(const nsCOMArray& aQueries, if (aOptions->MaxResults() > 0) return 0; + // RESULTS_AS_TAG_CONTENTS is quite similar to a folder shortcut, but we must + // avoid treating it like that, since we need to retain all query options. + if(aOptions->ResultType() == nsINavHistoryQueryOptions::RESULTS_AS_TAG_CONTENTS) + return 0; + // Note that we don't care about the onlyBookmarked flag, if you specify a bookmark // folder, onlyBookmarked is inferred. NS_ASSERTION(query->Folders()[0] > 0, "bad folder id"); diff --git a/mozilla/toolkit/components/places/src/nsNavHistoryQuery.cpp b/mozilla/toolkit/components/places/src/nsNavHistoryQuery.cpp index 44a406f28ed..31cdaeb0ae1 100644 --- a/mozilla/toolkit/components/places/src/nsNavHistoryQuery.cpp +++ b/mozilla/toolkit/components/places/src/nsNavHistoryQuery.cpp @@ -1195,7 +1195,7 @@ nsNavHistoryQueryOptions::GetResultType(PRUint16* aType) NS_IMETHODIMP nsNavHistoryQueryOptions::SetResultType(PRUint16 aType) { - if (aType > RESULTS_AS_TAG_QUERY) + if (aType > RESULTS_AS_TAG_CONTENTS) return NS_ERROR_INVALID_ARG; mResultType = aType; return NS_OK; diff --git a/mozilla/toolkit/components/places/src/nsNavHistoryResult.cpp b/mozilla/toolkit/components/places/src/nsNavHistoryResult.cpp index f98b97f1073..19f4b041ec4 100644 --- a/mozilla/toolkit/components/places/src/nsNavHistoryResult.cpp +++ b/mozilla/toolkit/components/places/src/nsNavHistoryResult.cpp @@ -2068,6 +2068,13 @@ nsNavHistoryQueryResultNode::nsNavHistoryQueryResultNode( PRBool nsNavHistoryQueryResultNode::CanExpand() { + if (IsContainersQuery()) + return PR_TRUE; + + // if we are child of an ExcludeItems root, we should not expand + if (mResult && mResult->mRootNode->mOptions->ExcludeItems()) + return PR_FALSE; + nsNavHistoryQueryOptions* options = GetGeneratingOptions(); if (options) { if (options->ExcludeItems()) @@ -2080,6 +2087,20 @@ nsNavHistoryQueryResultNode::CanExpand() return PR_FALSE; } +// nsNavHistoryQueryResultNode::IsContainersQuery +// +// Some query with a particular result type can contain other queries, +// they must be always expandable + +PRBool +nsNavHistoryQueryResultNode::IsContainersQuery() +{ + PRUint16 resultType = mOptions->ResultType(); + return resultType == nsINavHistoryQueryOptions::RESULTS_AS_DATE_QUERY || + resultType == nsINavHistoryQueryOptions::RESULTS_AS_DATE_SITE_QUERY || + resultType == nsINavHistoryQueryOptions::RESULTS_AS_TAG_QUERY || + resultType == nsINavHistoryQueryOptions::RESULTS_AS_SITE_QUERY; +} // nsNavHistoryQueryResultNode::OnRemoving // @@ -2140,10 +2161,49 @@ nsNavHistoryQueryResultNode::OpenContainer() NS_IMETHODIMP nsNavHistoryQueryResultNode::GetHasChildren(PRBool* aHasChildren) { - if (! CanExpand()) { + if (!CanExpand()) { *aHasChildren = PR_FALSE; return NS_OK; } + + PRUint16 resultType = mOptions->ResultType(); + // For tag containers query we must check if we have any tag + if (resultType == nsINavHistoryQueryOptions::RESULTS_AS_TAG_QUERY) { + nsNavHistory* history = nsNavHistory::GetHistoryService(); + NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY); + mozIStorageConnection *dbConn = history->GetStorageConnection(); + + nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService(); + NS_ENSURE_TRUE(bookmarks, NS_ERROR_OUT_OF_MEMORY); + PRInt64 tagsFolderId; + nsresult rv = bookmarks->GetTagsFolder(&tagsFolderId); + NS_ENSURE_SUCCESS(rv, rv); + + nsCOMPtr hasTagsStatement; + rv = dbConn->CreateStatement(NS_LITERAL_CSTRING( + "SELECT id FROM moz_bookmarks WHERE parent = ?1 LIMIT 1"), + getter_AddRefs(hasTagsStatement)); + NS_ENSURE_SUCCESS(rv, rv); + rv = hasTagsStatement->BindInt64Parameter(0, tagsFolderId); + NS_ENSURE_SUCCESS(rv, rv); + + return hasTagsStatement->ExecuteStep(aHasChildren); + } + + // For history containers query we must check if we have any history + if (resultType == nsINavHistoryQueryOptions::RESULTS_AS_DATE_QUERY || + resultType == nsINavHistoryQueryOptions::RESULTS_AS_DATE_SITE_QUERY || + resultType == nsINavHistoryQueryOptions::RESULTS_AS_SITE_QUERY) { + nsNavHistory* history = nsNavHistory::GetHistoryService(); + NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY); + return history->GetHasHistoryEntries(aHasChildren); + } + + //XXX: For other containers queries we must: + // 1. If it's open, just check mChildren for containers + // 2. Else null the view (keep it in a var), open container, check mChildren + // for containers, close container, reset the view + if (mContentsValid) { *aHasChildren = (mChildren.Count() > 0); return NS_OK; @@ -2372,6 +2432,12 @@ nsNavHistoryQueryResultNode::ClearChildren(PRBool aUnregister) nsresult nsNavHistoryQueryResultNode::Refresh() { + // Some query can return other queries, in this case we could call Refresh + // for each child query causing a major slowdown. We should not refresh + // nested queries since we are already refreshing the containining one. + if (mOptions->ResultType() == nsINavHistoryQueryOptions::RESULTS_AS_TAG_CONTENTS) + return NS_OK; + // Ignore refreshes when there is a batch, EndUpdateBatch will do a refresh // to get all the changes. if (mBatchInProgress) @@ -2657,7 +2723,10 @@ nsNavHistoryQueryResultNode::OnTitleChanged(nsIURI* aURI, nsCAutoString newTitle = NS_ConvertUTF16toUTF8(aPageTitle); PRBool onlyOneEntry = (mOptions->ResultType() == - nsINavHistoryQueryOptions::RESULTS_AS_URI); + nsINavHistoryQueryOptions::RESULTS_AS_URI || + mOptions->ResultType() == + nsINavHistoryQueryOptions::RESULTS_AS_TAG_CONTENTS + ); return ChangeTitles(aURI, newTitle, PR_TRUE, onlyOneEntry); } @@ -2671,7 +2740,9 @@ NS_IMETHODIMP nsNavHistoryQueryResultNode::OnDeleteURI(nsIURI *aURI) { PRBool onlyOneEntry = (mOptions->ResultType() == - nsINavHistoryQueryOptions::RESULTS_AS_URI); + nsINavHistoryQueryOptions::RESULTS_AS_URI || + mOptions->ResultType() == + nsINavHistoryQueryOptions::RESULTS_AS_TAG_CONTENTS); nsCAutoString spec; nsresult rv = aURI->GetSpec(spec); NS_ENSURE_SUCCESS(rv, rv); @@ -2736,7 +2807,9 @@ nsNavHistoryQueryResultNode::OnPageChanged(nsIURI *aURI, PRUint32 aWhat, case nsINavHistoryObserver::ATTRIBUTE_FAVICON: { nsCString newFavicon = NS_ConvertUTF16toUTF8(aValue); PRBool onlyOneEntry = (mOptions->ResultType() == - nsINavHistoryQueryOptions::RESULTS_AS_URI); + nsINavHistoryQueryOptions::RESULTS_AS_URI || + mOptions->ResultType() == + nsINavHistoryQueryOptions::RESULTS_AS_TAG_CONTENTS); UpdateURIs(PR_TRUE, onlyOneEntry, PR_FALSE, spec, setFaviconCallback, &newFavicon); break; @@ -2810,6 +2883,7 @@ nsNavHistoryQueryResultNode::OnItemVisited(PRInt64 aItemId, NS_NOTREACHED("history observers should not get OnItemVisited, but should get OnVisit instead"); return NS_OK; } + NS_IMETHODIMP nsNavHistoryQueryResultNode::OnItemMoved(PRInt64 aFolder, PRInt64 aOldParent, PRInt32 aOldIndex, PRInt64 aNewParent, diff --git a/mozilla/toolkit/components/places/src/nsNavHistoryResult.h b/mozilla/toolkit/components/places/src/nsNavHistoryResult.h index 7c08f5a6e82..4fd12359b63 100644 --- a/mozilla/toolkit/components/places/src/nsNavHistoryResult.h +++ b/mozilla/toolkit/components/places/src/nsNavHistoryResult.h @@ -706,6 +706,7 @@ public: NS_DECL_NSINAVHISTORYQUERYRESULTNODE PRBool CanExpand(); + PRBool IsContainersQuery(); virtual nsresult OpenContainer(); diff --git a/mozilla/toolkit/components/places/src/utils.js b/mozilla/toolkit/components/places/src/utils.js index ade656b9099..ea9fe24db75 100644 --- a/mozilla/toolkit/components/places/src/utils.js +++ b/mozilla/toolkit/components/places/src/utils.js @@ -255,8 +255,10 @@ var PlacesUtils = { nodeIsReadOnly: function PU_nodeIsReadOnly(aNode) { if (this.nodeIsFolder(aNode)) return this.bookmarks.getFolderReadonly(asQuery(aNode).folderItemId); - if (this.nodeIsQuery(aNode)) - return asQuery(aNode).childrenReadOnly; + if (this.nodeIsQuery(aNode) && + asQuery(aNode).queryOptions.resultType != + Ci.nsINavHistoryQueryOptions.RESULTS_AS_TAG_CONTENTS) + return aNode.childrenReadOnly; return false; }, @@ -288,6 +290,18 @@ var PlacesUtils = { resultType == Ci.nsINavHistoryQueryOptions.RESULTS_AS_DATE_SITE_QUERY); }, + /** + * Determines whether or not a result-node is a tag container. + * @param aNode + * A result-node + * @returns true if the node is a tag container, false otherwise + */ + nodeIsTagQuery: function PU_nodeIsTagQuery(aNode) { + return aNode.type == Ci.nsINavHistoryResultNode.RESULT_TYPE_QUERY && + asQuery(aNode).queryOptions.resultType == + Ci.nsINavHistoryQueryOptions.RESULTS_AS_TAG_CONTENTS; + }, + /** * Determines whether or not a ResultNode is a container. * @param aNode @@ -376,6 +390,13 @@ var PlacesUtils = { getConcreteItemId: function PU_getConcreteItemId(aNode) { if (aNode.type == Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER_SHORTCUT) return asQuery(aNode).folderItemId; + else if (PlacesUtils.nodeIsTagQuery(aNode)) { + // RESULTS_AS_TAG_CONTENTS queries are similar to folder shortcuts + // so we can still get the concrete itemId for them. + var queries = aNode.getQueries({}); + var folders = queries[0].getFolders({}); + return folders[0]; + } return aNode.itemId; }, diff --git a/mozilla/toolkit/components/places/tests/unit/test_419731.js b/mozilla/toolkit/components/places/tests/unit/test_419731.js new file mode 100644 index 00000000000..13b675eef34 --- /dev/null +++ b/mozilla/toolkit/components/places/tests/unit/test_419731.js @@ -0,0 +1,128 @@ +/* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim:set ts=2 sw=2 sts=2 et: */ +/* ***** BEGIN LICENSE BLOCK ***** + * Version: MPL 1.1/GPL 2.0/LGPL 2.1 + * + * The contents of this file are subject to the Mozilla Public License Version + * 1.1 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * http://www.mozilla.org/MPL/ + * + * Software distributed under the License is distributed on an "AS IS" basis, + * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License + * for the specific language governing rights and limitations under the + * License. + * + * The Original Code is Places unit test code. + * + * The Initial Developer of the Original Code is + * Mozilla Corporation. + * Portions created by the Initial Developer are Copyright (C) 2008 + * the Initial Developer. All Rights Reserved. + * + * Contributor(s): + * Marco Bonardo (Original Author) + * + * Alternatively, the contents of this file may be used under the terms of + * either the GNU General Public License Version 2 or later (the "GPL"), or + * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), + * in which case the provisions of the GPL or the LGPL are applicable instead + * of those above. If you wish to allow use of your version of this file only + * under the terms of either the GPL or the LGPL, and not to allow others to + * use your version of this file under the terms of the MPL, indicate your + * decision by deleting the provisions above and replace them with the notice + * and other provisions required by the GPL or the LGPL. If you do not delete + * the provisions above, a recipient may use your version of this file under + * the terms of any one of the MPL, the GPL or the LGPL. + * + * ***** END LICENSE BLOCK ***** */ + +// Get history services +try { + var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"]. + getService(Ci.nsINavHistoryService); + var bhist = histsvc.QueryInterface(Ci.nsIBrowserHistory); +} catch(ex) { + do_throw("Could not get history services\n"); +} + +// Get bookmark service +try { + var bmsvc = Cc["@mozilla.org/browser/nav-bookmarks-service;1"]. + getService(Ci.nsINavBookmarksService); +} +catch(ex) { + do_throw("Could not get the nav-bookmarks-service\n"); +} + +// Get tagging service +try { + var tagssvc = Cc["@mozilla.org/browser/tagging-service;1"]. + getService(Ci.nsITaggingService); +} catch(ex) { + do_throw("Could not get tagging service\n"); +} + + +// main +function run_test() { + var uri1 = uri("http://foo.bar/"); + + // create 2 bookmarks + var bookmark1id = bmsvc.insertBookmark(bmsvc.bookmarksMenuFolder, uri1, + bmsvc.DEFAULT_INDEX, "title 1"); + var bookmark2id = bmsvc.insertBookmark(bmsvc.toolbarFolder, uri1, + bmsvc.DEFAULT_INDEX, "title 2"); + // add a new tag + tagssvc.tagURI(uri1, ["foo"]); + + // get tag folder id + var options = histsvc.getNewQueryOptions(); + var query = histsvc.getNewQuery(); + query.setFolders([bmsvc.tagsFolder], 1); + var result = histsvc.executeQuery(query, options); + var tagRoot = result.root; + tagRoot.containerOpen = true; + var tag1node = tagRoot.getChild(0) + .QueryInterface(Ci.nsINavHistoryContainerResultNode); + var tag1itemId = tag1node.itemId; + tagRoot.containerOpen = false; + + // change bookmark 1 title + bmsvc.setItemTitle(bookmark1id, "new title 1"); + + // check that tag container contains new title + options = histsvc.getNewQueryOptions(); + options.queryType = Ci.nsINavHistoryQueryOptions.QUERY_TYPE_BOOKMARKS; + options.resultType = options.RESULTS_AS_TAG_CONTENTS; + options.includeHidden = true; + query = histsvc.getNewQuery(); + result = histsvc.executeQuery(query, options); + var root = result.root; + + root.containerOpen = true; + var cc = root.childCount; + do_check_eq(cc, 1); + var node = root.getChild(0); + do_check_eq(node.title, "new title 1"); + root.containerOpen = false; + + // change bookmark 2 title + bmsvc.setItemTitle(bookmark2id, "new title 2"); + + // check that tag container contains new title + options = histsvc.getNewQueryOptions(); + options.queryType = Ci.nsINavHistoryQueryOptions.QUERY_TYPE_BOOKMARKS; + options.resultType = options.RESULTS_AS_TAG_CONTENTS; + options.includeHidden = true; + query = histsvc.getNewQuery(); + result = histsvc.executeQuery(query, options); + root = result.root; + + root.containerOpen = true; + cc = root.childCount; + do_check_eq(cc, 1); + var node = root.getChild(0); + do_check_eq(node.title, "new title 2"); + root.containerOpen = false; +}