This test fails when browser.places.useAsyncTransactions is set to true: 3004 23:05:36 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_bug581253.js | the bookmark for the test url has been removed - 3009 23:05:36 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_bug581253.js | star button indicates that the bookmark has been removed - Got 1, expected 0

This test is still failing post bug 1371677, but just needs a simple rewrite so that it can deal with async transactions properly.

Comment on attachment 8885651 [details] Bug 1377600 - Rewrite browser_bug581253.js to handle be able to handle the new async PlacesTransactions UI. I just realised, I should stop this using the old transaction manager. I don't think it is actually necessary for the test from what I can tell - as we just need a bookmark to remove, we're not pre-loading the star UI as such.

Comment on attachment 8885651 [details] Bug 1377600 - Rewrite browser_bug581253.js to handle be able to handle the new async PlacesTransactions UI. https://reviewboard.mozilla.org/r/156504/#review161602 ::: browser/base/content/test/general/browser_bug581253.js:10 (Diff revision 2) > var testTag = "581253_tag"; > -var timerID = -1; > > -function test() { > - registerCleanupFunction(function() { > +add_task(async function test_remove_bookmark_with_tag_via_edit_bookmark() { > + registerCleanupFunction(async function() { > PlacesUtils.bookmarks.removeFolderChildren(PlacesUtils.unfiledBookmarksFolderId); can we use eraseEverything here? my only doubt is whether cleaning up the other roots may cause failures in the next tests... ::: browser/base/content/test/general/browser_bug581253.js:11 (Diff revision 2) > -var timerID = -1; > > -function test() { > - registerCleanupFunction(function() { > +add_task(async function test_remove_bookmark_with_tag_via_edit_bookmark() { > + registerCleanupFunction(async function() { > PlacesUtils.bookmarks.removeFolderChildren(PlacesUtils.unfiledBookmarksFolderId); > - if (timerID > 0) { > + gBrowser.removeCurrentTab(); I'd suggest to use await BrowserTestUtils.removeTab, so in the future it may be easier to fix only one code point. ::: browser/base/content/test/general/browser_bug581253.js:52 (Diff revision 2) > -} > > -function onPanelHidden(aEvent) { > - if (aEvent.target == StarUI.panel) { > - StarUI.panel.removeEventListener("popuphidden", arguments.callee); > - > + await popupHiddenPromise; > + await BrowserTestUtils.waitForCondition(async () => { > + return !(await PlacesUtils.bookmarks.fetch({url: testURL})); > + }, "the bookmark for the test url has been removed"); I'd prefer waitForNotification for onItemRemoved

Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7a8d5e183e80 Rewrite browser_bug581253.js to handle be able to handle the new async PlacesTransactions UI. r=mak