nagai/fune
3
0
Fork 0
forked from mirrors/gecko-dev
Code Pull requests Activity

Bug 1376929 - Fix places-related mochitests in chrome/ for async-transactions. r=standard8

Browse source 
MozReview-Commit-ID: ILrvOGzu1zo

--HG--
extra : rebase_source : de00f7f734711e2d2c8521626ad8857ab858bb97
This commit is contained in:
Marco Bonardo 2017-07-03 15:46:56 +02:00
parent b9179e9d33
commit d3bad38937
11 changed files with 125 additions and 60 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

6
addon-sdk/source/lib/sdk/places/host/host-tags.js
View file

@ -37,7 +37,8 @@ function tag (message) {
event: message.event
};
resData.data = taggingService.tagURI(newURI(data.url), data.tags);
if (data.tags && data.tags.length > 0)
resData.data = taggingService.tagURI(newURI(data.url), data.tags);
respond(resData);
}
@ -48,7 +49,8 @@ function untag (message) {
event: message.event
};
resData.data = taggingService.untagURI(newURI(data.url), data.tags);
if (!data.tags || data.tags.length > 0)
resData.data = taggingService.untagURI(newURI(data.url), data.tags);
respond(resData);
}

13
browser/components/places/content/editBookmarkOverlay.js
View file

@ -511,7 +511,10 @@ var gEditItemOverlay = {
if (aCurrentTags.length == 0)
return { newTags: inputTags, removedTags: [] };
let removedTags = aCurrentTags.filter(t => !inputTags.includes(t));
// Do not remove tags that may be reinserted with a different
// case, since the tagging service may handle those more efficiently.
let lcInputTags = inputTags.map(t => t.toLowerCase());
let removedTags = aCurrentTags.filter(t => !lcInputTags.includes(t.toLowerCase()));
let newTags = inputTags.filter(t => !aCurrentTags.includes(t));
return { removedTags, newTags };
},
@ -537,14 +540,14 @@ var gEditItemOverlay = {
}
let setTags = async function() {
if (removedTags.length > 0) {
await PlacesTransactions.Untag({ urls: aURIs, tags: removedTags })
.transact();
}
if (newTags.length > 0) {
await PlacesTransactions.Tag({ urls: aURIs, tags: newTags })
.transact();
}
if (removedTags.length > 0) {
await PlacesTransactions.Untag({ urls: aURIs, tags: removedTags })
.transact();
}
};
// Only in the library info-pane it's safe (and necessary) to batch these.

4
browser/components/places/tests/browser/browser_bookmarkProperties_editTagContainer.js
View file

@ -31,7 +31,7 @@ add_task(async function() {
Assert.ok(tree.controller.isCommandEnabled("placesCmd_show:info"),
"'placesCmd_show:info' on current selected node is enabled");
let promiseTitleResetNotification = promiseBookmarksNotification(
let promiseTitleResetNotification = PlacesTestUtils.waitForNotification(
"onItemChanged", (itemId, prop, isAnno, val) => prop == "title" && val == "tag1");
await withBookmarksDialog(
@ -48,7 +48,7 @@ add_task(async function() {
Assert.ok(!namepicker.readOnly, "Name field should not be read-only");
Assert.equal(namepicker.value, "tag1", "Node title is correct");
let promiseTitleChangeNotification = promiseBookmarksNotification(
let promiseTitleChangeNotification = PlacesTestUtils.waitForNotification(
"onItemChanged", (itemId, prop, isAnno, val) => prop == "title" && val == "tag2");
fillBookmarkTextField("editBMPanel_namePicker", "tag2", dialogWin);

12
browser/components/places/tests/chrome/test_bug485100-change-case-loses-tag.xul
View file

@ -28,6 +28,7 @@
src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js" />
<script type="application/javascript"
src="chrome://browser/content/places/editBookmarkOverlay.js"/>
<script type="application/javascript" src="head.js" />
<body xmlns="http://www.w3.org/1999/xhtml" />
@ -35,10 +36,8 @@
<script type="application/javascript">
<![CDATA[
function runTest() {
SimpleTest.waitForExplicitFinish();
(async function() {
let testTag = "foo";
let testTagUpper = "Foo";
@ -60,14 +59,23 @@
// add a tag
document.getElementById("editBMPanel_tagsField").value = testTag;
let promiseNotification = PlacesTestUtils.waitForNotification(
"onItemChanged", (id, property) => property == "tags");
gEditItemOverlay.onTagsFieldChange();
await promiseNotification;
// test that the tag has been added in the backend
is(PlacesUtils.tagging.getTagsForURI(testURI)[0], testTag, "tags match");
// change the tag
document.getElementById("editBMPanel_tagsField").value = testTagUpper;
// The old sync API doesn't notify a tags change, and fixing it would be
// quite complex, so we just wait for a title change until tags are
// refactored.
promiseNotification = PlacesTestUtils.waitForNotification(
"onItemChanged", (id, property) => property == "title");
gEditItemOverlay.onTagsFieldChange();
await promiseNotification;
// test that the tag has been added in the backend
is(PlacesUtils.tagging.getTagsForURI(testURI)[0], testTagUpper, "tags match");

35
browser/components/places/tests/chrome/test_bug631374_tags_selector_scroll.xul
View file

@ -27,6 +27,7 @@
src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js" />
<script type="application/javascript"
src="chrome://browser/content/places/editBookmarkOverlay.js"/>
<script type="application/javascript" src="head.js" />
<body xmlns="http://www.w3.org/1999/xhtml" />
@ -34,7 +35,6 @@
<script type="application/javascript">
<![CDATA[
/**
* This test checks that editing tags doesn't scroll the tags selector
* listbox to wrong positions.
@ -42,19 +42,15 @@
function runTest() {
SimpleTest.waitForExplicitFinish();
(async function() {
let bs = PlacesUtils.bookmarks;
await PlacesUtils.bookmarks.eraseEverything();
let tags = ["a", "b", "c", "d", "e", "f", "g",
"h", "i", "l", "m", "n", "o", "p"];
// Add a bookmark and tag it.
let uri1 = Services.io.newURI("http://www1.mozilla.org/");
let bm1 = await bs.insert({
parentGuid: bs.toolbarGuid,
index: bs.DEFAULT_INDEX,
type: bs.TYPE_BOOKMARK,
let bm1 = await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
title: "mozilla",
url: uri1.spec
});
@ -62,10 +58,8 @@
// Add a second bookmark so that tags won't disappear when unchecked.
let uri2 = Services.io.newURI("http://www2.mozilla.org/");
let bm2 = await bs.insert({
parentGuid: bs.toolbarGuid,
index: bs.DEFAULT_INDEX,
type: bs.TYPE_BOOKMARK,
let bm2 = await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
title: "mozilla",
url: uri2.spec
});
@ -93,7 +87,10 @@
let selectedTag = listItem.label;
// Uncheck the tag.
let promiseNotification = PlacesTestUtils.waitForNotification(
"onItemChanged", (id, property) => property == "tags");
listItem.checked = false;
await promiseNotification;
is(visibleIndex, tagsSelector.getIndexOfFirstVisibleRow(),
"Scroll position did not change");
@ -104,13 +101,16 @@
is(newItem.label, selectedTag, "Correct tag is still selected");
// Check the tag.
promiseNotification = PlacesTestUtils.waitForNotification(
"onItemChanged", (id, property) => property == "tags");
newItem.checked = true;
await promiseNotification;
is(visibleIndex, tagsSelector.getIndexOfFirstVisibleRow(),
"Scroll position did not change");
}
// Remove the second bookmark, then nuke some of the tags.
await bs.remove(bm2.guid);
await PlacesUtils.bookmarks.remove(bm2);
// Doing this backwords tests more interesting paths.
for (let i = tags.length - 1; i >= 0 ; i -= 2) {
@ -125,7 +125,10 @@
let selectedTag = listItem.label;
// Uncheck the tag.
let promiseNotification = PlacesTestUtils.waitForNotification(
"onItemChanged", (id, property) => property == "tags");
listItem.checked = false;
await promiseNotification;
// Ensure the first visible tag is still visible in the list.
let firstVisibleIndex = tagsSelector.getIndexOfFirstVisibleRow();
@ -145,8 +148,8 @@
}
// Cleanup.
await bs.remove(bm1.guid);
})().then(SimpleTest.finish).catch(alert);
await PlacesUtils.bookmarks.remove(bm1);
})().catch(ex => ok(false, "test failed: " + ex)).then(SimpleTest.finish);
}
function openTagSelector() {
@ -158,10 +161,8 @@
resolve();
});
});
// Open the tags selector.
document.getElementById("editBMPanel_tagsSelectorExpander").doCommand();
return promise;
}
]]>

24
toolkit/components/places/Bookmarks.jsm
View file

@ -232,7 +232,7 @@ var Bookmarks = Object.freeze({
// If it's a tag, notify OnItemChanged to all bookmarks for this URL.
if (isTagging) {
for (let entry of (await fetchBookmarksByURL(item))) {
for (let entry of (await fetchBookmarksByURL(item, true))) {
notify(observers, "onItemChanged", [ entry._id, "tags", false, "",
PlacesUtils.toPRTime(entry.lastModified),
entry.type, entry._parentId,
@ -642,6 +642,7 @@ var Bookmarks = Object.freeze({
updatedItem.source ]);
}
if (updateInfo.hasOwnProperty("title")) {
let isTagging = updatedItem.parentGuid == Bookmarks.tagsGuid;
notify(observers, "onItemChanged", [ updatedItem._id, "title",
false, updatedItem.title,
PlacesUtils.toPRTime(updatedItem.lastModified),
@ -649,7 +650,22 @@ var Bookmarks = Object.freeze({
updatedItem._parentId,
updatedItem.guid,
updatedItem.parentGuid, "",
updatedItem.source ]);
updatedItem.source ],
{ isTagging });
// If we're updating a tag, we must notify all the tagged bookmarks
// about the change.
if (isTagging) {
let URIs = PlacesUtils.tagging.getURIsForTag(updatedItem.title);
for (let uri of URIs) {
for (let entry of (await fetchBookmarksByURL({ url: new URL(uri.spec) }, true))) {
notify(observers, "onItemChanged", [ entry._id, "tags", false, "",
PlacesUtils.toPRTime(entry.lastModified),
entry.type, entry._parentId,
entry.guid, entry.parentGuid,
"", updatedItem.source ]);
}
}
}
}
if (updateInfo.hasOwnProperty("url")) {
notify(observers, "onItemChanged", [ updatedItem._id, "uri",
@ -738,7 +754,7 @@ var Bookmarks = Object.freeze({
{ isTagging: isUntagging });
if (isUntagging) {
for (let entry of (await fetchBookmarksByURL(item))) {
for (let entry of (await fetchBookmarksByURL(item, true))) {
notify(observers, "onItemChanged", [ entry._id, "tags", false, "",
PlacesUtils.toPRTime(entry.lastModified),
entry.type, entry._parentId,
@ -2224,7 +2240,7 @@ async function(db, folderGuids, options) {
let isUntagging = item._grandParentId == PlacesUtils.tagsFolderId;
if (isUntagging) {
for (let entry of (await fetchBookmarksByURL(item))) {
for (let entry of (await fetchBookmarksByURL(item, true))) {
notify(observers, "onItemChanged", [ entry._id, "tags", false, "",
PlacesUtils.toPRTime(entry.lastModified),
entry.type, entry._parentId,

6
toolkit/components/places/PlacesSyncUtils.jsm
View file

@ -1420,9 +1420,11 @@ function tagItem(item, tags) {
// tag IDs, we temporarily tag a dummy URI, ensuring the tags exist.
let dummyURI = PlacesUtils.toURI("about:weave#BStore_tagURI");
let bookmarkURI = PlacesUtils.toURI(item.url.href);
PlacesUtils.tagging.tagURI(dummyURI, newTags, SOURCE_SYNC);
if (newTags && newTags.length > 0)
PlacesUtils.tagging.tagURI(dummyURI, newTags, SOURCE_SYNC);
PlacesUtils.tagging.untagURI(bookmarkURI, null, SOURCE_SYNC);
PlacesUtils.tagging.tagURI(bookmarkURI, newTags, SOURCE_SYNC);
if (newTags && newTags.length > 0)
PlacesUtils.tagging.tagURI(bookmarkURI, newTags, SOURCE_SYNC);
PlacesUtils.tagging.untagURI(dummyURI, null, SOURCE_SYNC);
return newTags;

14
toolkit/components/places/PlacesTransactions.jsm
View file

@ -1270,7 +1270,7 @@ PT.EditUrl.prototype = Object.seal({
PlacesUtils.tagging.untagURI(originalURI, originalTags);
let currentNewURITags = PlacesUtils.tagging.getTagsForURI(uri);
newURIAdditionalTags = originalTags.filter(t => !currentNewURITags.includes(t));
if (newURIAdditionalTags)
if (newURIAdditionalTags && newURIAdditionalTags.length > 0)
PlacesUtils.tagging.tagURI(uri, newURIAdditionalTags);
}
}
@ -1559,12 +1559,18 @@ PT.Untag.prototype = {
} else {
tagsToRemove = tagsSet;
}
PlacesUtils.tagging.untagURI(uri, tagsToRemove);
if (tagsToRemove.length > 0) {
PlacesUtils.tagging.untagURI(uri, tagsToRemove);
}
onUndo.unshift(() => {
PlacesUtils.tagging.tagURI(uri, tagsToRemove);
if (tagsToRemove.length > 0) {
PlacesUtils.tagging.tagURI(uri, tagsToRemove);
}
});
onRedo.push(() => {
PlacesUtils.tagging.untagURI(uri, tagsToRemove);
if (tagsToRemove.length > 0) {
PlacesUtils.tagging.untagURI(uri, tagsToRemove);
}
});
}
this.undo = async function() {

26
toolkit/components/places/nsNavBookmarks.cpp
View file

@ -1987,19 +1987,19 @@ nsNavBookmarks::SetItemTitle(int64_t aItemId, const nsACString& aTitle,
NS_ENSURE_SUCCESS(rv, rv);
}
NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
nsINavBookmarkObserver,
OnItemChanged(bookmark.id,
NS_LITERAL_CSTRING("title"),
false,
title,
bookmark.lastModified,
bookmark.type,
bookmark.parentId,
bookmark.guid,
bookmark.parentGuid,
EmptyCString(),
aSource));
NOTIFY_BOOKMARKS_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
SKIP_TAGS(isChangingTagFolder),
OnItemChanged(bookmark.id,
NS_LITERAL_CSTRING("title"),
false,
title,
bookmark.lastModified,
bookmark.type,
bookmark.parentId,
bookmark.guid,
bookmark.parentGuid,
EmptyCString(),
aSource));
return NS_OK;
}

20
toolkit/components/places/nsTaggingService.js
View file

@ -128,7 +128,7 @@ TaggingService.prototype = {
// have created it.
tag.__defineGetter__("id", () => this._getItemIdForTag(tag.name));
} else {
throw Cr.NS_ERROR_INVALID_ARG;
throw Components.Exception("Invalid tag value", Cr.NS_ERROR_INVALID_ARG);
}
return tag;
});
@ -136,8 +136,8 @@ TaggingService.prototype = {
// nsITaggingService
tagURI: function TS_tagURI(aURI, aTags, aSource) {
if (!aURI || !aTags || !Array.isArray(aTags)) {
throw Cr.NS_ERROR_INVALID_ARG;
if (!aURI || !aTags || !Array.isArray(aTags) || aTags.length == 0) {
throw Components.Exception("Invalid value for tags", Cr.NS_ERROR_INVALID_ARG);
}
// This also does some input validation.
@ -215,8 +215,8 @@ TaggingService.prototype = {
// nsITaggingService
untagURI: function TS_untagURI(aURI, aTags, aSource) {
if (!aURI || (aTags && !Array.isArray(aTags))) {
throw Cr.NS_ERROR_INVALID_ARG;
if (!aURI || (aTags && (!Array.isArray(aTags) || aTags.length == 0))) {
throw Components.Exception("Invalid value for tags", Cr.NS_ERROR_INVALID_ARG);
}
if (!aTags) {
@ -257,8 +257,9 @@ TaggingService.prototype = {
// nsITaggingService
getURIsForTag: function TS_getURIsForTag(aTagName) {
if (!aTagName || aTagName.length == 0)
throw Cr.NS_ERROR_INVALID_ARG;
if (!aTagName || aTagName.length == 0) {
throw Components.Exception("Invalid tag name", Cr.NS_ERROR_INVALID_ARG);
}
if (/^\s|\s$/.test(aTagName)) {
Deprecated.warning("Tag passed to getURIsForTag was not trimmed",
@ -293,8 +294,9 @@ TaggingService.prototype = {
// nsITaggingService
getTagsForURI: function TS_getTagsForURI(aURI, aCount) {
if (!aURI)
throw Cr.NS_ERROR_INVALID_ARG;
if (!aURI) {
throw Components.Exception("Invalid uri", Cr.NS_ERROR_INVALID_ARG);
}
let tags = [];
let db = PlacesUtils.history.DBConnection;

25
toolkit/components/places/tests/PlacesTestUtils.jsm
View file

@ -325,4 +325,29 @@ this.PlacesTestUtils = Object.freeze({
dateRemoved: PlacesUtils.toDate(row.getResultByName("dateRemoved")),
}));
},
waitForNotification(notification, conditionFn = () => true, type = "bookmarks") {
let iface = type == "bookmarks" ? Ci.nsINavBookmarkObserver
: Ci.nsINavHistoryObserver;
return new Promise(resolve => {
let proxifiedObserver = new Proxy({}, {
get: (target, name) => {
if (name == "QueryInterface")
return XPCOMUtils.generateQI([iface]);
if (name == notification)
return (...args) => {
if (conditionFn.apply(this, args)) {
PlacesUtils[type].removeObserver(proxifiedObserver);
resolve();
}
}
if (name == "skipTags" || name == "skipDescendantsOnItemRemoval") {
return false;
}
return () => false;
}
});
PlacesUtils[type].addObserver(proxifiedObserver);
});
},
});