496266 - unit test for bug 496103 (browser_drag_bookmarks_on_toolbar.js) causes page navigation

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[:Gavin Sharp [email: [e-mail]]]

The bookmark drag test was causing a page navigation, which in turn was causing a failure in browser_library_left_pane_commands.js (there were 2 history entries rather than 1 to delete):

TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/places/tests/browser/browser_library_left_pane_commands.js | Visit has been removed
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/places/tests/browser/browser_library_left_pane_commands.js | Visit has been removed

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1244087343.1244090418.14714.gz&fulltext=1

https://hg.mozilla.org/mozilla-central/rev/69b64ba05f94

Comment 1

[Marco Bonardo [:mak]]

Attachment: patch v1.0

Prevent click event, this will avoid the visit.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 381481 [details] [diff] [review]
patch v1.0

I applied that patch, and then reverted m-c revision 69b64ba05f94 (the disabling of the second test):

http://pastebin.mozilla.org/655320

Unfortunately browser_library_left_pane_commands.js still fails when all browser/components/places tests are run at once:

python obj-ff-opt/_tests/testing/mochitest/runtests.py --browser-chrome --test-path=browser/components/places/

and the test still causes mozilla.org to be loaded.

Comment 3

[Marco Bonardo [:mak]]

hm, i tested running all browser-chrome tests without any problem and the page was loading before the patch and not loading after it... that's really strange

Comment 4

[Marco Bonardo [:mak]]

hm, could be i've attached wrong patch since the one i recall included removing the early return.

Comment 5

[Marco Bonardo [:mak]]

Attachment: patch v1.1

the strange thing is that this patch (that is basically what you have tested) works for me on both windows and linux (With patch in bug 496277). So i'm trying with the tryserver since my mac env is not yet fully functional.

Comment 6

[Marco Bonardo [:mak]]

Comment on attachment 381655 [details] [diff] [review]
patch v1.1

trybuilds have not shown any failure in Places tests or in left_pane_commands test, could you please retry?

Comment 7

[:Gavin Sharp [email: [e-mail]]]

I've tested that patch again, and it seems to still cause a page navigation (easy to notice by running it alone). I was able to get the entire directory of tests to pass once, but a subsequent run failed in browser_library_left_pane_commands.js again (as in comment 0).

Comment 8

[Marco Bonardo [:mak]]

2 subsequent trybuilds have not shown any problem though. nor i can reproduce running the test tens of times on win. my mac env is up and running so i should be able to test there subsequent runs now.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 381655 [details] [diff] [review]
patch v1.1

I can't reproduce the failures I was seeing earlier, oddly enough, so this will do!

Comment 10

[Marco Bonardo [:mak]]

I've run the test about 10 times alone and 10 times the full Places browser-chrome harness, i always got green. Something strange must be happened there, eventually unit test boxes will tell us.
I'll wait for m-c full reopen before pushing these, since i want to avoid creating issues to blockers that are still landing.

Comment 11

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/194348da49d1