Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-11-29 07:42:04 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 458026 (part 2) - Do not allow creation of empty-named tags. r=mano sr=rs a=blocking

Browse Source
This commit is contained in:
Marco Bonardo 2010-10-16 11:06:35 +02:00
parent 6dde196d7a
commit 8cb19660cc
7 changed files with 219 additions and 110 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
browser/components/places/content/editBookmarkOverlay.js
View File

@ -692,7 +692,12 @@ var gEditItemOverlay = {
// Here we update either the item title or its cached static title
var newTitle = this._element("userEnteredName").label;
if (this._getItemStaticTitle() != newTitle) {
if (!newTitle &&
PlacesUtils.bookmarks.getFolderIdForItem(this._itemId) == PlacesUtils.tagsFolderId) {
// We don't allow setting an empty title for a tag, restore the old one.
this._initNamePicker();
}
else if (this._getItemStaticTitle() != newTitle) {
this._mayUpdateFirstEditField("namePicker");
if (PlacesUtils.microsummaries.hasMicrosummary(this._itemId)) {
// Note: this implicitly also takes care of the microsummary->static
@ -1003,20 +1008,16 @@ var gEditItemOverlay = {
}
},
/**
* Splits "tagsField" element value, returning an array of valid tag strings.
*
* @return Array of tag strings found in the field value.
*/
_getTagsArrayFromTagField: function EIO__getTagsArrayFromTagField() {
// we don't require the leading space (after each comma)
var tags = this._element("tagsField").value.split(",");
for (var i=0; i < tags.length; i++) {
// remove trailing and leading spaces
tags[i] = tags[i].replace(/^\s+/, "").replace(/\s+$/, "");
// remove empty entries from the array.
if (tags[i] == "") {
tags.splice(i, 1);
i--;
}
}
return tags;
let tags = this._element("tagsField").value;
return tags.trim()
.split(/\s*,\s*/) // Split on commas and remove spaces.
.filter(function (tag) tag.length > 0); // Kill empty tags.
},
newFolder: function EIO_newFolder() {

12
toolkit/components/places/public/nsITaggingService.idl
View File

@ -52,9 +52,9 @@ interface nsITaggingService : nsISupports
* @param aURI
* the URL to tag.
* @param aTags
* Array of tags to set for the given URL. Each element within the
* array can be either a tag name or a concrete itemId of a tag
* container.
* Array of tags to set for the given URL. Each element within the
* array can be either a tag name (non-empty string) or a concrete
* itemId of a tag container.
*/
void tagURI(in nsIURI aURI, in nsIVariant aTags);
@ -65,9 +65,9 @@ interface nsITaggingService : nsISupports
* @param aURI
* the URL to un-tag.
* @param aTags
* Array of tags to unset. pass null to remove all tags from the given
* url. Each element within the array can be either a tag name or a
* concrete itemId of a tag container.
* Array of tags to unset. Pass null to remove all tags from the given
* url. Each element within the array can be either a tag name
* (non-empty string) or a concrete itemId of a tag container.
*/
void untagURI(in nsIURI aURI, in nsIVariant aTags);

13
toolkit/components/places/src/PlacesDBUtils.jsm
View File

@ -457,6 +457,19 @@ nsPlacesDBUtils.prototype = {
removeLivemarkStaticItems.params["lmfailed"] = "about:livemark-failed";
cleanupStatements.push(removeLivemarkStaticItems);
// D.12 Fix empty-named tags.
// Tags were allowed to have empty names due to a UI bug. Fix them
// replacing their title with "(notitle)".
let fixEmptyNamedTags = this._dbConn.createStatement(
"UPDATE moz_bookmarks SET title = :empty_title " +
"WHERE length(title) = 0 AND type = :folder_type " +
"AND parent = :tags_folder"
);
fixEmptyNamedTags.params["empty_title"] = "(notitle)";
fixEmptyNamedTags.params["folder_type"] = this._bms.TYPE_FOLDER;
fixEmptyNamedTags.params["tags_folder"] = this._bms.tagsFolder;
cleanupStatements.push(fixEmptyNamedTags);
// MOZ_FAVICONS
// E.1 remove orphan icons
let deleteOrphanIcons = this._dbConn.createStatement(

135
toolkit/components/places/src/nsTaggingService.js
View File

@ -139,47 +139,79 @@ TaggingService.prototype = {
return -1;
},
/**
* Makes a proper array of tag objects like { id: number, name: string }.
*
* @param aTags
* Array of tags. Entries can be tag names or concrete item id.
* @return Array of tag objects like { id: number, name: string }.
*
* @throws Cr.NS_ERROR_INVALID_ARG if any element of the input array is not
* a valid tag.
*/
_convertInputMixedTagsArray: function TS__convertInputMixedTagsArray(aTags)
{
return aTags.map(function (val)
{
let tag = { _self: this };
if (typeof(val) == "number" && this._tagFolders[val]) {
// This is a tag folder id.
tag.id = val;
// We can't know the name at this point, since a previous tag could
// want to change it.
tag.__defineGetter__("name", function () this._self._tagFolders[this.id]);
}
else if (typeof(val) == "string" && val.length > 0) {
// This is a tag name.
tag.name = val;
// We can't know the id at this point, since a previous tag could
// have created it.
tag.__defineGetter__("id", function () this._self._getItemIdForTag(this.name));
}
else {
throw Cr.NS_ERROR_INVALID_ARG;
}
return tag;
}, this);
},
// nsITaggingService
tagURI: function TS_tagURI(aURI, aTags) {
if (!aURI || !aTags)
tagURI: function TS_tagURI(aURI, aTags)
{
if (!aURI || !aTags || !Array.isArray(aTags)) {
throw Cr.NS_ERROR_INVALID_ARG;
}
// This also does some input validation.
let tags = this._convertInputMixedTagsArray(aTags);
let taggingService = this;
PlacesUtils.bookmarks.runInBatchMode({
_self: this,
runBatched: function(aUserData) {
for (var i = 0; i < aTags.length; i++) {
var tag = aTags[i];
var tagId = null;
if (typeof(tag) == "number") {
// is it a tag folder id?
if (this._self._tagFolders[tag]) {
tagId = tag;
tag = this._self._tagFolders[tagId];
}
else
throw Cr.NS_ERROR_INVALID_ARG;
}
else {
tagId = this._self._getItemIdForTag(tag);
if (tagId == -1)
tagId = this._self._createTag(tag);
runBatched: function (aUserData)
{
tags.forEach(function (tag)
{
if (tag.id == -1) {
// Tag does not exist yet, create it.
tag.id = this._createTag(tag.name);
}
var itemId = this._self._getItemIdForTaggedURI(aURI, tag);
if (itemId == -1) {
if (this._getItemIdForTaggedURI(aURI, tag.name) == -1) {
// The provided URI is not yet tagged, add a tag for it.
// Note that bookmarks under tag containers must have null titles.
PlacesUtils.bookmarks.insertBookmark(
tagId, aURI, PlacesUtils.bookmarks.DEFAULT_INDEX, null
tag.id, aURI, PlacesUtils.bookmarks.DEFAULT_INDEX, null
);
}
// Rename the tag container so the Places view would match the
// most-recent user-typed values.
var currentTagTitle = PlacesUtils.bookmarks.getItemTitle(tagId);
if (currentTagTitle != tag) {
PlacesUtils.bookmarks.setItemTitle(tagId, tag);
this._self._tagFolders[tagId] = tag;
// Try to preserve user's tag name casing.
// Rename the tag container so the Places view matches the most-recent
// user-typed value.
if (PlacesUtils.bookmarks.getItemTitle(tag.id) != tag.name) {
// this._tagFolders is updated by the bookmarks observer.
PlacesUtils.bookmarks.setItemTitle(tag.id, tag.name);
}
}
}, taggingService);
}
}, null);
},
@ -204,42 +236,37 @@ TaggingService.prototype = {
},
// nsITaggingService
untagURI: function TS_untagURI(aURI, aTags) {
if (!aURI)
untagURI: function TS_untagURI(aURI, aTags)
{
if (!aURI || (aTags && !Array.isArray(aTags))) {
throw Cr.NS_ERROR_INVALID_ARG;
}
if (!aTags) {
// see IDL.
// Passing null should clear all tags for aURI, see the IDL.
// XXXmano: write a perf-sensitive version of this code path...
aTags = this.getTagsForURI(aURI);
}
PlacesUtils.bookmarks.runInBatchMode({
_self: this,
runBatched: function(aUserData) {
for (var i = 0; i < aTags.length; i++) {
var tag = aTags[i];
var tagId = null;
if (typeof(tag) == "number") {
// is it a tag folder id?
if (this._self._tagFolders[tag]) {
tagId = tag;
tag = this._self._tagFolders[tagId];
}
else
throw Cr.NS_ERROR_INVALID_ARG;
}
else
tagId = this._self._getItemIdForTag(tag);
// This also does some input validation.
let tags = this._convertInputMixedTagsArray(aTags);
if (tagId != -1) {
var itemId = this._self._getItemIdForTaggedURI(aURI, tag);
let taggingService = this;
PlacesUtils.bookmarks.runInBatchMode({
runBatched: function (aUserData)
{
tags.forEach(function (tag)
{
if (tag.id != -1) {
// A tag could exist.
let itemId = this._getItemIdForTaggedURI(aURI, tag.name);
if (itemId != -1) {
// There is a tagged item.
PlacesUtils.bookmarks.removeItem(itemId);
this._self._removeTagIfEmpty(tagId);
this._removeTagIfEmpty(tag.id);
}
}
}
}, taggingService);
}
}, null);
},

5
toolkit/components/places/tests/unit/test_adaptive.js
View File

@ -152,8 +152,9 @@ function setCountRank(aURI, aCount, aRank, aSearch, aBookmark)
"test_book");
// And add the tag if we need to.
if (aBookmark == "tag")
PlacesUtils.tagging.tagURI(aURI, "test_tag");
if (aBookmark == "tag") {
PlacesUtils.tagging.tagURI(aURI, ["test_tag"]);
}
}
}

54
toolkit/components/places/tests/unit/test_preventive_maintenance.js
View File

@ -95,15 +95,16 @@ function addPlace(aUrl, aFavicon) {
return mDBConn.lastInsertRowID;
}
function addBookmark(aPlaceId, aType, aParent, aKeywordId, aFolderType) {
function addBookmark(aPlaceId, aType, aParent, aKeywordId, aFolderType, aTitle) {
let stmt = mDBConn.createStatement(
"INSERT INTO moz_bookmarks (fk, type, parent, keyword_id, folder_type) " +
"VALUES (:place_id, :type, :parent, :keyword_id, :folder_type)");
"INSERT INTO moz_bookmarks (fk, type, parent, keyword_id, folder_type, title) " +
"VALUES (:place_id, :type, :parent, :keyword_id, :folder_type, :title)");
stmt.params["place_id"] = aPlaceId || null;
stmt.params["type"] = aType || bs.TYPE_BOOKMARK;
stmt.params["parent"] = aParent || bs.unfiledBookmarksFolder;
stmt.params["keyword_id"] = aKeywordId || null;
stmt.params["folder_type"] = aFolderType || null;
stmt.params["title"] = typeof(aTitle) == "string" ? aTitle : null;
stmt.execute();
stmt.finalize();
return mDBConn.lastInsertRowID;
@ -762,6 +763,53 @@ tests.push({
//------------------------------------------------------------------------------
tests.push({
name: "D.12",
desc: "Fix empty-named tags",
setup: function() {
// Add a place to ensure place_id = 1 is valid
let placeId = addPlace();
// Create a empty-named tag.
this._untitledTagId = addBookmark(null, bs.TYPE_FOLDER, bs.tagsFolder, null, null, "");
// Insert a bookmark in the tag, otherwise it will be removed.
addBookmark(placeId, bs.TYPE_BOOKMARK, this._untitledTagId);
// Create a empty-named folder.
this._untitledFolderId = addBookmark(null, bs.TYPE_FOLDER, bs.toolbarFolder, null, null, "");
// Create a titled tag.
this._titledTagId = addBookmark(null, bs.TYPE_FOLDER, bs.tagsFolder, null, null, "titledTag");
// Insert a bookmark in the tag, otherwise it will be removed.
addBookmark(placeId, bs.TYPE_BOOKMARK, this._titledTagId);
// Create a titled folder.
this._titledFolderId = addBookmark(null, bs.TYPE_FOLDER, bs.toolbarFolder, null, null, "titledFolder");
},
check: function() {
// Check that valid bookmark is still there
let stmt = mDBConn.createStatement(
"SELECT title FROM moz_bookmarks WHERE id = :id"
);
stmt.params["id"] = this._untitledTagId;
do_check_true(stmt.executeStep());
do_check_eq(stmt.row.title, "(notitle)");
stmt.reset();
stmt.params["id"] = this._untitledFolderId;
do_check_true(stmt.executeStep());
do_check_eq(stmt.row.title, "");
stmt.reset();
stmt.params["id"] = this._titledTagId;
do_check_true(stmt.executeStep());
do_check_eq(stmt.row.title, "titledTag");
stmt.reset();
stmt.params["id"] = this._titledFolderId;
do_check_true(stmt.executeStep());
do_check_eq(stmt.row.title, "titledFolder");
stmt.finalize();
}
});
//------------------------------------------------------------------------------
tests.push({
name: "E.1",
desc: "Remove orphan icons",

81
toolkit/components/places/tests/unit/test_tagging.js
View File

@ -37,40 +37,17 @@
*
* ***** END LICENSE BLOCK ***** */
// Get history service
try {
var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"].
getService(Ci.nsINavHistoryService);
} catch(ex) {
do_throw("Could not get history service\n");
}
// Notice we use createInstance because later we will have to terminate the
// service and restart it.
var tagssvc = Cc["@mozilla.org/browser/tagging-service;1"].
createInstance().QueryInterface(Ci.nsITaggingService);
// 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 {
// Notice we use createInstance because later we will have to terminate the
// service and restart it.
var tagssvc = Cc["@mozilla.org/browser/tagging-service;1"].
createInstance().QueryInterface(Ci.nsITaggingService);
} catch(ex) {
do_throw("Could not get tagging service\n");
}
// main
function run_test() {
var options = histsvc.getNewQueryOptions();
var query = histsvc.getNewQuery();
var options = PlacesUtils.history.getNewQueryOptions();
var query = PlacesUtils.history.getNewQuery();
query.setFolders([bmsvc.tagsFolder], 1);
var result = histsvc.executeQuery(query, options);
query.setFolders([PlacesUtils.tagsFolderId], 1);
var result = PlacesUtils.history.executeQuery(query, options);
var tagRoot = result.root;
tagRoot.containerOpen = true;
@ -172,6 +149,48 @@ function run_test() {
do_check_true(uri4Tags.indexOf("tag 3") != -1);
do_check_true(uri4Tags.indexOf("456") != -1);
// Test sparse arrays.
let (curChildCount = tagRoot.childCount) {
try {
tagssvc.tagURI(uri1, [, "tagSparse"]);
do_check_eq(tagRoot.childCount, curChildCount + 1);
} catch (ex) {
do_throw("Passing a sparse array should not throw");
}
try {
tagssvc.untagURI(uri1, [, "tagSparse"]);
do_check_eq(tagRoot.childCount, curChildCount);
} catch (ex) {
do_throw("Passing a sparse array should not throw");
}
// Test that the API throws for bad arguments.
try {
tagssvc.tagURI(uri1, ["", "test"]);
do_throw("Passing a bad tags array should throw");
} catch (ex) {
do_check_eq(ex.name, "NS_ERROR_ILLEGAL_VALUE");
}
try {
tagssvc.untagURI(uri1, ["", "test"]);
do_throw("Passing a bad tags array should throw");
} catch (ex) {
do_check_eq(ex.name, "NS_ERROR_ILLEGAL_VALUE");
}
try {
tagssvc.tagURI(uri1, [0, "test"]);
do_throw("Passing a bad tags array should throw");
} catch (ex) {
do_check_eq(ex.name, "NS_ERROR_ILLEGAL_VALUE");
}
try {
tagssvc.tagURI(uri1, [0, "test"]);
do_throw("Passing a bad tags array should throw");
} catch (ex) {
do_check_eq(ex.name, "NS_ERROR_ILLEGAL_VALUE");
}
}
// cleanup
tagRoot.containerOpen = false;
}