1377599 - browser_bookmark_popup.js fails with async places transactions turned on

Status: RESOLVED FIXED

(Toolkit :: Places, defect)

Description

[Mark Banner (:standard8)]

This test fails when browser.places.useAsyncTransactions is set to true:

23:03:39 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_bookmark_popup.js | Page is starred after closing - Got true, expected false
2347 23:03:39 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_bookmark_popup.js | bookmark should not be present if a panel action should've removed it - Got true, expected false
2361 23:03:39 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_bookmark_popup.js | Page is starred after closing - Got true, expected false
2366 23:03:39 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_bookmark_popup.js | bookmark should not be present if a panel action should've removed it - Got true, expected false
2380 23:03:41 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_bookmark_popup.js | Page is starred after closing - Got true, expected false
2385 23:03:41 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_bookmark_popup.js | bookmark should not be present if a panel action should've removed it - Got true, expected false
2406 23:03:43 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_bookmark_popup.js | Page is starred after closing - Got true, expected false
2411 23:03:43 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_bookmark_popup.js | bookmark should not be present if a panel action should've removed it - Got true, expected false

Comment 1

[Mark Banner (:standard8)]

I've found the issue here as well - there's some mistakes in the Star UI for bookmark removal, and the test needs to wait for the bookmark remove notification.

Comment 2

[Mark Banner (:standard8)]

Attachment: Bug 1377599 - Fix typos to make the Star UI remove bookmarks correctly, and add an extra hook to ensure the tests passes in async mode.

Review commit: https://reviewboard.mozilla.org/r/156580/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/156580/

Comment 3

[Marco Bonardo [:mak]]

Comment on attachment 8885790 [details]
Bug 1377599 - Fix typos to make the Star UI remove bookmarks correctly, and add an extra hook to ensure the tests passes in async mode.

https://reviewboard.mozilla.org/r/156580/#review161666

I assume you are checking these on Try, I can't tell if other tests may me unhappy off-hand. Good work and good catch on the PlacesUtils typo!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/813550f3f713
Fix typos to make the Star UI remove bookmarks correctly, and add an extra hook to ensure the tests passes in async mode. r=mak

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/813550f3f713