Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Sortable Folder and Path Columns to the Bookmarks Search #843

Merged
merged 4 commits into from
Jan 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions browser/components/places/content/places.xul
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,13 @@
<splitter class="tree-splitter"/>
<treecol label="&col.lastmodified.label;" id="placesContentLastModified" anonid="lastModified" flex="1" hidden="true"
persist="width hidden ordinal sortActive sortDirection"/>
<splitter class="tree-splitter"/>
<treecol label="&col.parentfolder.label;" id="placesContentParentFolder" anonid="parentFolder" flex="1" hidden="true"
persist="width hidden ordinal sortActive sortDirection"/>
<splitter class="tree-splitter"/>
<treecol label="&col.parentpath.label;" id="placesContentParentPath" anonid="parentPath" flex="1" hidden="true"
persist="width hidden ordinal sortActive sortDirection"/>

</treecols>
<treechildren flex="1" onclick="ContentTree.onClick(event);"/>
</tree>
Expand Down
30 changes: 30 additions & 0 deletions browser/components/places/content/treeView.js
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,8 @@ PlacesTreeView.prototype = {
COLUMN_TYPE_DATEADDED: 6,
COLUMN_TYPE_LASTMODIFIED: 7,
COLUMN_TYPE_TAGS: 8,
COLUMN_TYPE_PARENTFOLDER: 9,
COLUMN_TYPE_PARENTPATH: 10,

_getColumnType: function PTV__getColumnType(aColumn) {
let columnType = aColumn.element.getAttribute("anonid") || aColumn.id;
Expand All @@ -603,6 +605,10 @@ PlacesTreeView.prototype = {
return this.COLUMN_TYPE_LASTMODIFIED;
case "tags":
return this.COLUMN_TYPE_TAGS;
case "parentFolder":
return this.COLUMN_TYPE_PARENTFOLDER;
case "parentPath":
return this.COLUMN_TYPE_PARENTPATH;
}
return this.COLUMN_TYPE_UNKNOWN;
},
Expand Down Expand Up @@ -1555,6 +1561,10 @@ PlacesTreeView.prototype = {
if (node.lastModified)
return this._convertPRTimeToString(node.lastModified);
return "";
case this.COLUMN_TYPE_PARENTFOLDER:
return node.parentFolder
case this.COLUMN_TYPE_PARENTPATH:
return node.parentPath
}
return "";
},
Expand Down Expand Up @@ -1717,6 +1727,26 @@ PlacesTreeView.prototype = {
newSort = NHQO.SORT_BY_TAGS_ASCENDING;

break;
case this.COLUMN_TYPE_PARENTFOLDER:
if (oldSort == NHQO.SORT_BY_PARENTFOLDER_ASCENDING)
newSort = NHQO.SORT_BY_PARENTFOLDER_DESCENDING;
else if (allowTriState && oldSort == NHQO.SORT_BY_PARENTFOLDER_DESCENDING)
newSort = NHQO.SORT_BY_NONE;
else
newSort = NHQO.SORT_BY_PARENTFOLDER_ASCENDING;

break;
case this.COLUMN_TYPE_PARENTPATH:
if (oldSort == NHQO.SORT_BY_PARENTPATH_ASCENDING)
newSort = NHQO.SORT_BY_PARENTPATH_DESCENDING;
else if (allowTriState && oldSort == NHQO.SORT_BY_PARENTPATH_DESCENDING)
newSort = NHQO.SORT_BY_NONE;
else
newSort = NHQO.SORT_BY_PARENTPATH_ASCENDING;

break;


default:
throw Cr.NS_ERROR_INVALID_ARG;
}
Expand Down
2 changes: 2 additions & 0 deletions browser/locales/en-US/chrome/browser/places/places.dtd
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@
<!ENTITY col.description.label "Description">
<!ENTITY col.dateadded.label "Added">
<!ENTITY col.lastmodified.label "Last Modified">
<!ENTITY col.parentfolder.label "Parent Folder">
<!ENTITY col.parentpath.label "Parent Path">

<!ENTITY search.placeholder "Search">

Expand Down
13 changes: 11 additions & 2 deletions toolkit/components/places/nsINavHistoryService.idl
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,11 @@ interface nsINavHistoryResultNode : nsISupports
* history visit nodes, this value is 0.
*/
readonly attribute unsigned long visitType;

readonly attribute AUTF8String parentFolder;

readonly attribute AUTF8String parentPath;

};


Expand Down Expand Up @@ -1018,8 +1023,12 @@ interface nsINavHistoryQueryOptions : nsISupports
const unsigned short SORT_BY_TAGS_DESCENDING = 18;
const unsigned short SORT_BY_ANNOTATION_ASCENDING = 19;
const unsigned short SORT_BY_ANNOTATION_DESCENDING = 20;
const unsigned short SORT_BY_FRECENCY_ASCENDING = 21;
const unsigned short SORT_BY_FRECENCY_DESCENDING = 22;
const unsigned short SORT_BY_PARENTFOLDER_ASCENDING = 21;
const unsigned short SORT_BY_PARENTFOLDER_DESCENDING = 22;
const unsigned short SORT_BY_PARENTPATH_ASCENDING = 23;
const unsigned short SORT_BY_PARENTPATH_DESCENDING = 24;
const unsigned short SORT_BY_FRECENCY_ASCENDING = 25;
const unsigned short SORT_BY_FRECENCY_DESCENDING = 26;

/**
* "URI" results, one for each URI visited in the range. Individual result
Expand Down
12 changes: 7 additions & 5 deletions toolkit/components/places/nsNavBookmarks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@
using namespace mozilla;

// These columns sit to the right of the kGetInfoIndex_* columns.
const int32_t nsNavBookmarks::kGetChildrenIndex_Guid = 18;
const int32_t nsNavBookmarks::kGetChildrenIndex_Position = 19;
const int32_t nsNavBookmarks::kGetChildrenIndex_Type = 20;
const int32_t nsNavBookmarks::kGetChildrenIndex_PlaceID = 21;
const int32_t nsNavBookmarks::kGetChildrenIndex_SyncStatus = 22;
const int32_t nsNavBookmarks::kGetChildrenIndex_Guid = 20;
const int32_t nsNavBookmarks::kGetChildrenIndex_Position = 21;
const int32_t nsNavBookmarks::kGetChildrenIndex_Type = 22;
const int32_t nsNavBookmarks::kGetChildrenIndex_PlaceID = 23;
const int32_t nsNavBookmarks::kGetChildrenIndex_SyncStatus = 24;

using namespace mozilla::places;

Expand Down Expand Up @@ -1126,6 +1126,7 @@ nsNavBookmarks::GetDescendantChildren(int64_t aFolderId,
"SELECT h.id, h.url, b.title, h.rev_host, h.visit_count, "
"h.last_visit_date, null, b.id, b.dateAdded, b.lastModified, "
"b.parent, null, h.frecency, h.hidden, h.guid, null, null, null, "
"null, null, "
"b.guid, b.position, b.type, b.fk, b.syncStatus "
"FROM moz_bookmarks b "
"LEFT JOIN moz_places h ON b.fk = h.id "
Expand Down Expand Up @@ -2172,6 +2173,7 @@ nsNavBookmarks::QueryFolderChildren(
"SELECT h.id, h.url, b.title, h.rev_host, h.visit_count, "
"h.last_visit_date, null, b.id, b.dateAdded, b.lastModified, "
"b.parent, null, h.frecency, h.hidden, h.guid, null, null, null, "
"null, null, "
"b.guid, b.position, b.type, b.fk "
"FROM moz_bookmarks b "
"LEFT JOIN moz_places h ON b.fk = h.id "
Expand Down
96 changes: 73 additions & 23 deletions toolkit/components/places/nsNavHistory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,17 @@ void GetTagsSqlFragment(int64_t aTagsFolder,

_sqlFragment.AppendLiteral(" AS tags ");
}
#define PathQuerySql(tagsFolder) ( NS_LITERAL_CSTRING(" \
with recursive folder_path(id,title) as \
(select m.id, m.title from moz_bookmarks m \
where m.parent is 0 \
union \
select m.id, p.title || '/' || m.title \
from moz_bookmarks as m,folder_path as p \
where m.parent = p.id and m.type is 2 and m.id <> ") \
+ nsPrintfCString("%" PRId64, tagsFolder) + NS_LITERAL_CSTRING(" \
) \
") )

/**
* This class sets begin/end of batch updates to correspond to C++ scopes so
Expand Down Expand Up @@ -261,12 +272,14 @@ const int32_t nsNavHistory::kGetInfoIndex_Guid = 14;
const int32_t nsNavHistory::kGetInfoIndex_VisitId = 15;
const int32_t nsNavHistory::kGetInfoIndex_FromVisitId = 16;
const int32_t nsNavHistory::kGetInfoIndex_VisitType = 17;
const int32_t nsNavHistory::kGetInfoIndex_ParentFolder = 18;
const int32_t nsNavHistory::kGetInfoIndex_ParentPath = 19;
// These columns are followed by corresponding constants in nsNavBookmarks.cpp,
// which must be kept in sync:
// nsNavBookmarks::kGetChildrenIndex_Guid = 18;
// nsNavBookmarks::kGetChildrenIndex_Position = 19;
// nsNavBookmarks::kGetChildrenIndex_Type = 20;
// nsNavBookmarks::kGetChildrenIndex_PlaceID = 21;
// nsNavBookmarks::kGetChildrenIndex_Guid = 20;
// nsNavBookmarks::kGetChildrenIndex_Position = 21;
// nsNavBookmarks::kGetChildrenIndex_Type = 22;
// nsNavBookmarks::kGetChildrenIndex_PlaceID = 23;

PLACES_FACTORY_SINGLETON_IMPLEMENTATION(nsNavHistory, gHistoryService)

Expand Down Expand Up @@ -1517,7 +1530,8 @@ PlacesSQLQueryBuilder::SelectAsURI()
"SELECT h.id, h.url, h.title AS page_title, h.rev_host, h.visit_count, "
"h.last_visit_date, null, null, null, null, null, ") +
tagsSqlFragment + NS_LITERAL_CSTRING(", h.frecency, h.hidden, h.guid, "
"null, null, null "
"null, null, null, "
"null,null "
"FROM moz_places h "
// WHERE 1 is a no-op since additonal conditions will start with AND.
"WHERE 1 "
Expand All @@ -1538,12 +1552,15 @@ PlacesSQLQueryBuilder::SelectAsURI()
mHasSearchTerms,
tagsSqlFragment);

mQueryString = NS_LITERAL_CSTRING(
mQueryString = PathQuerySql(history->GetTagsFolder()) +
NS_LITERAL_CSTRING(
"SELECT b2.fk, h.url, b2.title AS page_title, "
"h.rev_host, h.visit_count, h.last_visit_date, null, b2.id, "
"b2.dateAdded, b2.lastModified, b2.parent, ") +
tagsSqlFragment + NS_LITERAL_CSTRING(", h.frecency, h.hidden, h.guid, "
"null, null, null, b2.guid, b2.position, b2.type, b2.fk "
"null, null, null, "
"null, null "
",b2.guid, b2.position, b2.type, b2.fk "
"FROM moz_bookmarks b2 "
"JOIN (SELECT b.fk "
"FROM moz_bookmarks b "
Expand All @@ -1562,15 +1579,17 @@ PlacesSQLQueryBuilder::SelectAsURI()
NS_LITERAL_CSTRING("b.fk"),
mHasSearchTerms,
tagsSqlFragment);
mQueryString = NS_LITERAL_CSTRING(
mQueryString = PathQuerySql(history->GetTagsFolder()) + NS_LITERAL_CSTRING(
"SELECT b.fk, h.url, b.title AS page_title, "
"h.rev_host, h.visit_count, h.last_visit_date, null, b.id, "
"b.dateAdded, b.lastModified, b.parent, ") +
tagsSqlFragment + NS_LITERAL_CSTRING(", h.frecency, h.hidden, h.guid,"
"null, null, null, b.guid, b.position, b.type, b.fk "
"FROM moz_bookmarks b "
"null, null, null, "
"f.title, p.title "
",b.guid, b.position, b.type, b.fk "
"FROM moz_bookmarks f, moz_bookmarks b, folder_path p "
"JOIN moz_places h ON b.fk = h.id "
"WHERE NOT EXISTS "
"WHERE f.id = b.parent AND f.id = p.id AND NOT EXISTS "
"(SELECT id FROM moz_bookmarks "
"WHERE id = b.parent AND parent = ") +
nsPrintfCString("%" PRId64, history->GetTagsFolder()) +
Expand Down Expand Up @@ -1599,7 +1618,8 @@ PlacesSQLQueryBuilder::SelectAsVisit()
"SELECT h.id, h.url, h.title AS page_title, h.rev_host, h.visit_count, "
"v.visit_date, null, null, null, null, null, ") +
tagsSqlFragment + NS_LITERAL_CSTRING(", h.frecency, h.hidden, h.guid, "
"v.id, v.from_visit, v.visit_type "
"v.id, v.from_visit, v.visit_type, "
"null, null "
"FROM moz_places h "
"JOIN moz_historyvisits v ON h.id = v.place_id "
// WHERE 1 is a no-op since additonal conditions will start with AND.
Expand Down Expand Up @@ -1634,7 +1654,8 @@ PlacesSQLQueryBuilder::SelectAsDay()
"SELECT null, "
"'place:type=%d&sort=%d&beginTime='||beginTime||'&endTime='||endTime, "
"dayTitle, null, null, beginTime, null, null, null, null, null, null, "
"null, null, null "
"null, null, null, "
"null, null "
"FROM (", // TOUTER BEGIN
resultType,
sortingMode);
Expand Down Expand Up @@ -1837,7 +1858,8 @@ PlacesSQLQueryBuilder::SelectAsSite()
mQueryString = nsPrintfCString(
"SELECT null, 'place:type=%d&sort=%d&domain=&domainIsHost=true'%s, "
":localhost, :localhost, null, null, null, null, null, null, null, "
"null, null, null "
"null, null, null, "
"null, null "
"WHERE EXISTS ( "
"SELECT h.id FROM moz_places h "
"%s "
Expand All @@ -1852,7 +1874,8 @@ PlacesSQLQueryBuilder::SelectAsSite()
"SELECT null, "
"'place:type=%d&sort=%d&domain='||host||'&domainIsHost=true'%s, "
"host, host, null, null, null, null, null, null, null, "
"null, null, null "
"null, null, null, "
"null, null "
"FROM ( "
"SELECT get_unreversed_host(h.rev_host) AS host "
"FROM moz_places h "
Expand Down Expand Up @@ -1892,7 +1915,8 @@ PlacesSQLQueryBuilder::SelectAsTag()
mQueryString = nsPrintfCString(
"SELECT null, 'place:folder=' || id || '&queryType=%d&type=%d', "
"title, null, null, null, null, null, dateAdded, "
"lastModified, null, null, null, null, null, null "
"lastModified, null, null, null, null, null, null, "
"null, null "
"FROM moz_bookmarks "
"WHERE parent = %" PRId64,
nsINavHistoryQueryOptions::QUERY_TYPE_BOOKMARKS,
Expand Down Expand Up @@ -2040,6 +2064,18 @@ PlacesSQLQueryBuilder::OrderBy()
case nsINavHistoryQueryOptions::SORT_BY_ANNOTATION_ASCENDING:
case nsINavHistoryQueryOptions::SORT_BY_ANNOTATION_DESCENDING:
break; // Sort later in nsNavHistoryQueryResultNode::FillChildren()
case nsINavHistoryQueryOptions::SORT_BY_PARENTFOLDER_ASCENDING:
OrderByColumnIndexAsc(nsNavHistory::kGetInfoIndex_ParentFolder);
break;
case nsINavHistoryQueryOptions::SORT_BY_PARENTFOLDER_DESCENDING:
OrderByColumnIndexDesc(nsNavHistory::kGetInfoIndex_ParentFolder);
break;
case nsINavHistoryQueryOptions::SORT_BY_PARENTPATH_ASCENDING:
OrderByColumnIndexAsc(nsNavHistory::kGetInfoIndex_ParentPath);
break;
case nsINavHistoryQueryOptions::SORT_BY_PARENTPATH_DESCENDING:
OrderByColumnIndexDesc(nsNavHistory::kGetInfoIndex_ParentPath);
break;
case nsINavHistoryQueryOptions::SORT_BY_FRECENCY_ASCENDING:
OrderByColumnIndexAsc(nsNavHistory::kGetInfoIndex_Frecency);
break;
Expand Down Expand Up @@ -2127,7 +2163,8 @@ nsNavHistory::ConstructQueryString(
"SELECT h.id, h.url, h.title AS page_title, h.rev_host, h.visit_count, h.last_visit_date, "
"null, null, null, null, null, ") +
tagsSqlFragment + NS_LITERAL_CSTRING(", h.frecency, h.hidden, h.guid, "
"null, null, null "
"null, null, null, "
"null, null "
"FROM moz_places h "
"WHERE h.hidden = 0 "
"AND EXISTS (SELECT id FROM moz_historyvisits WHERE place_id = h.id "
Expand Down Expand Up @@ -3833,6 +3870,9 @@ nsNavHistory::RowToResult(mozIStorageValueArray* aRow,
// itemId
int64_t itemId = aRow->AsInt64(kGetInfoIndex_ItemId);
int64_t parentId = -1;
nsAutoCString parentFolder,parentPath;
parentFolder = "";
parentPath = "";
if (itemId == 0) {
// This is not a bookmark. For non-bookmarks we use a -1 itemId value.
// Notice ids in sqlite tables start from 1, so itemId cannot ever be 0.
Expand All @@ -3845,6 +3885,10 @@ nsNavHistory::RowToResult(mozIStorageValueArray* aRow,
// The Places root has parent == 0, but that item id does not really
// exist. We want to set the parent only if it's a real one.
parentId = itemParentId;
rv = aRow->GetUTF8String(kGetInfoIndex_ParentFolder, parentFolder);
NS_ENSURE_SUCCESS(rv, rv);
rv = aRow->GetUTF8String(kGetInfoIndex_ParentPath, parentPath);
NS_ENSURE_SUCCESS(rv, rv);
}
}

Expand Down Expand Up @@ -3897,6 +3941,8 @@ nsNavHistory::RowToResult(mozIStorageValueArray* aRow,
if (itemId != -1) {
resultNode->mItemId = itemId;
resultNode->mFolderId = parentId;
resultNode->mParentFolder = parentFolder;
resultNode->mParentPath = parentPath;
resultNode->mDateAdded = aRow->AsInt64(kGetInfoIndex_ItemDateAdded);
resultNode->mLastModified = aRow->AsInt64(kGetInfoIndex_ItemLastModified);

Expand Down Expand Up @@ -4057,7 +4103,8 @@ nsNavHistory::VisitIdToResultNode(int64_t visitId,
"SELECT h.id, h.url, h.title, h.rev_host, h.visit_count, "
"v.visit_date, null, null, null, null, null, "
) + tagsFragment + NS_LITERAL_CSTRING(", h.frecency, h.hidden, h.guid, "
"v.id, v.from_visit, v.visit_type "
"v.id, v.from_visit, v.visit_type, "
"null, null "
"FROM moz_places h "
"JOIN moz_historyvisits v ON h.id = v.place_id "
"WHERE v.id = :visit_id ")
Expand All @@ -4071,7 +4118,8 @@ nsNavHistory::VisitIdToResultNode(int64_t visitId,
"SELECT h.id, h.url, h.title, h.rev_host, h.visit_count, "
"h.last_visit_date, null, null, null, null, null, "
) + tagsFragment + NS_LITERAL_CSTRING(", h.frecency, h.hidden, h.guid, "
"null, null, null "
"null, null, null, "
"null, null "
"FROM moz_places h "
"JOIN moz_historyvisits v ON h.id = v.place_id "
"WHERE v.id = :visit_id ")
Expand Down Expand Up @@ -4112,15 +4160,17 @@ nsNavHistory::BookmarkIdToResultNode(int64_t aBookmarkId, nsNavHistoryQueryOptio
GetTagsSqlFragment(GetTagsFolder(), NS_LITERAL_CSTRING("h.id"),
true, tagsFragment);
// Should match kGetInfoIndex_*
nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(NS_LITERAL_CSTRING(
nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement( PathQuerySql(GetTagsFolder()) + NS_LITERAL_CSTRING(
"SELECT b.fk, h.url, b.title, "
"h.rev_host, h.visit_count, h.last_visit_date, null, b.id, "
"b.dateAdded, b.lastModified, b.parent, "
) + tagsFragment + NS_LITERAL_CSTRING(", h.frecency, h.hidden, h.guid, "
"null, null, null, b.guid, b.position, b.type, b.fk "
"FROM moz_bookmarks b "
"null, null, null, "
"f.title, p.title "
",b.guid, b.position, b.type, b.fk "
"FROM moz_bookmarks f, moz_bookmarks b, folder_path p "
"JOIN moz_places h ON b.fk = h.id "
"WHERE b.id = :item_id ")
"WHERE b.id = :item_id AND f.id = b.parent AND f.id = p.id")
);
NS_ENSURE_STATE(stmt);
mozStorageStatementScoper scoper(stmt);
Expand Down
3 changes: 2 additions & 1 deletion toolkit/components/places/nsNavHistory.h
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,8 @@ class nsNavHistory final : public nsSupportsWeakReference
static const int32_t kGetInfoIndex_VisitId;
static const int32_t kGetInfoIndex_FromVisitId;
static const int32_t kGetInfoIndex_VisitType;

static const int32_t kGetInfoIndex_ParentFolder;
static const int32_t kGetInfoIndex_ParentPath;
int64_t GetTagsFolder();

// this actually executes a query and gives you results, it is used by
Expand Down