735139 - Improve browser_pageInfo.js a little

Status: VERIFIED FIXED

(Firefox :: Page Info Window, defect, P4)

Comment 1

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) Improve browser_pageInfo.js a little [Checked in: See comment 3+5]

Comment 2

[Dão Gottwald [:dao]]

Comment on attachment 605294 [details] [diff] [review]
(Av1) Improve browser_pageInfo.js a little
[Checked in: See comment 3+5]

>-    ok(feedRowsNum == 3, "Number of feeds listed: " +
>-                         feedRowsNum + ", should be 3");
>-
>+    is(feedRowsNum, 3, "3 feeds listed");

is(feedRowsNum, 3, "Number of feeds listed");

>     for (var i = 0; i < feedRowsNum; i++) {
>       let feedItem = feedListbox.getItemAtIndex(i);
>-      ok(feedItem.getAttribute("name") == (i+1), 
>-         "Name given: " + feedItem.getAttribute("name") + ", should be " + (i+1));
>+      is(feedItem.getAttribute("name"), i + 1,
>+         "Feed name " + (i + 1) + "/" + feedRowsNum);

is(feedItem.getAttribute("name"), i + 1, "Feed name");

r+ with these changes

Comment 3

[Serge Gautherie (:sgautherie)]

Comment on attachment 605294 [details] [diff] [review]
(Av1) Improve browser_pageInfo.js a little
[Checked in: See comment 3+5]

Av1, with comment 2 suggestion(s).


(In reply to Dão Gottwald [:dao] from comment #2)

Ftr,

> >+    is(feedRowsNum, 3, "3 feeds listed");
> 
> is(feedRowsNum, 3, "Number of feeds listed");

My text change was per bug 521263 comment 26, but I'm fine retaining the initial text.

> >-      ok(feedItem.getAttribute("name") == (i+1), 
> >-         "Name given: " + feedItem.getAttribute("name") + ", should be " + (i+1));
> >+      is(feedItem.getAttribute("name"), i + 1,
> >+         "Feed name " + (i + 1) + "/" + feedRowsNum);
> 
> is(feedItem.getAttribute("name"), i + 1, "Feed name");

I'm not totally fine with having no reference to which name is checked (especially without hint at how many they should be) and getting 3 times the same text, but as you wish.

Comment 4

[Dão Gottwald [:dao]]

is() will spit out the expected value in case of a failure, which seems sufficient here.

Comment 5

[Serge Gautherie (:sgautherie)]

Comment on attachment 605294 [details] [diff] [review]
(Av1) Improve browser_pageInfo.js a little
[Checked in: See comment 3+5]

https://hg.mozilla.org/mozilla-central/rev/cf4978c2e32c

Comment 6

[Serge Gautherie (:sgautherie)]

https://tbpl.mozilla.org/php/getParsedLog.php?id=10037139&tree=Firefox&full=1
Rev3 Fedora 12x64 mozilla-central debug test mochitest-other on 2012-03-13 10:17:57 PDT for push cf4978c2e32c
{
TEST-PASS | [...] | Feed tab
TEST-PASS | [...] | Feed list
TEST-PASS | [...] | Number of feeds listed
TEST-PASS | [...] | Feed name
TEST-PASS | [...] | Feed name
TEST-PASS | [...] | Feed name
}

V.Fixed