557496 - Port Firefox bookmarks tests to SeaMonkey

Status: RESOLVED FIXED

(SeaMonkey :: Bookmarks & History, defect)

Description

[Robert Kaiser]

Once bug 498596 switches us to places bookmarks, we also should get tests for those, starting porting the Firefox tests over to SeaMonkey.

Comment 1

[Robert Kaiser]

Attachment: add those tests that succeed

This patch adds all the tests that succeed, I spent too many hours trying to make others work, esp. in the browser-chrome area, and I gave up on them for now.

Comment 2

[Justin Wood (:Callek)]

Comment on attachment 458244 [details] [diff] [review]
add those tests that succeed

rs+ based on a skim... but

>+++ b/suite/common/places/tree.xml
>-            for (let j = min.value; j <= max.value; ++j)
>-              nodes.push(resultview.nodeForTreeIndex(j));
>+            if (max.value > -1)
>+              for (let j = min.value; j <= max.value; ++j)
>+                nodes.push(resultview.nodeForTreeIndex(j));

Was this another patches remnant, or needed for a test here, if so can you please explain it for posterity.

(If I find time to actually review the tests, I may steal review from neil; but I would think my rs+ is ok for the test-only changes, especially if they all pass.)

Comment 3

[Robert Kaiser]

(In reply to comment #2)
> Was this another patches remnant, or needed for a test here, if so can you
> please explain it for posterity.

Thanks for catching that, it's indeed a remnant, though from testing why some of the not added tests failed. It's not wrong, but it probably doesn't belong there.

Comment 4

[Robert Kaiser]

Pushed as http://hg.mozilla.org/comm-central/rev/b9e980d6f85d

Comment 5

[Serge Gautherie (:sgautherie)]

(In reply to comment #3)
> It's not wrong, but it probably doesn't belong there.

This |if (max.value > -1)| got pushed anyway.
Should we revert it (ftb)?
Or try to add it to Firefox too?