Last Comment Bug 731663 - Port |Bug 593566 - Bookmarks with blank name are wrongly exported (broken codepage symbols in the exported file)| to SeaMonkey
: Port |Bug 593566 - Bookmarks with blank name are wrongly exported (broken cod...
Status: VERIFIED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Bookmarks & History (show other bugs)
: Trunk
: All All
: P4 minor (vote)
: seamonkey2.11
Assigned To: Serge Gautherie (:sgautherie)
:
Mentors:
http://mxr.mozilla.org/comm-central/f...
Depends on: 593566
Blocks: 730849
  Show dependency treegraph
 
Reported: 2012-02-29 09:46 PST by Serge Gautherie (:sgautherie)
Modified: 2012-04-23 19:45 PDT (History)
5 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
verified


Attachments
(Av1-SM) Port |Bug 593566 - Bookmarks with blank name are wrongly exported (broken codepage symbols in the exported file)| to SeaMonkey (31.14 KB, patch)
2012-02-29 12:47 PST, Serge Gautherie (:sgautherie)
no flags Details | Diff | Review
(Bv1-FF) test_bookmarks_html.js: Fix nits, Add a do_print() (6.92 KB, patch)
2012-02-29 13:06 PST, Serge Gautherie (:sgautherie)
mak77: review+
Details | Diff | Review
(Bv1a-FF) test_bookmarks_html.js: Fix nits, Add/Use do_print(), Improve documentation [Checked in: See comment 9] (8.25 KB, patch)
2012-03-01 04:21 PST, Serge Gautherie (:sgautherie)
mak77: review+
Details | Diff | Review
(Av1a-SM) Port |Bug 593566 - Bookmarks with blank name are wrongly exported (broken codepage symbols in the exported file)| to SeaMonkey [Checked in: See comment 12] (31.23 KB, patch)
2012-03-05 11:13 PST, Serge Gautherie (:sgautherie)
iann_bugzilla: review+
bugspam.Callek: approval‑comm‑aurora+
Details | Diff | Review
(Av1b-SM-a210) Port |Bug 593566 - Bookmarks with blank name are wrongly exported (broken codepage symbols in the exported file)| to SeaMonkey [Checkin: Comment 19] (29.64 KB, patch)
2012-04-16 15:06 PDT, Serge Gautherie (:sgautherie)
no flags Details | Diff | Review
(Cv1-SM-a210) Typo fix for patch Av1b-SM-a210 [Checkin: Comment 22] (1.54 KB, patch)
2012-04-17 20:26 PDT, Serge Gautherie (:sgautherie)
no flags Details | Diff | Review

Description Serge Gautherie (:sgautherie) 2012-02-29 09:46:59 PST
(Noticed while working on bug 730849.)
Comment 1 Serge Gautherie (:sgautherie) 2012-02-29 12:47:15 PST
Created attachment 601717 [details] [diff] [review]
(Av1-SM)  Port |Bug 593566 - Bookmarks with blank name are wrongly exported (broken codepage symbols in the exported file)| to SeaMonkey

Test code is identical, except some nits (see upcoming FF patch).
Test data are rewritten to match SeaMonkey existing test data file.

test_384370.js still passes.
http://mxr.mozilla.org/comm-central/search?string=bookmarks.preplaces.html&case=1&find=%2Ftest
Comment 2 Serge Gautherie (:sgautherie) 2012-02-29 13:06:16 PST
Created attachment 601728 [details] [diff] [review]
(Bv1-FF) test_bookmarks_html.js: Fix nits, Add a do_print()

testImportedBookmarksToFolder() assumes the menu items come first in the data file.
Is that a rule or a (bad) assumption in the test?

NB: I just noticed that my do_print() should probably be a print(), to match the rest of the file...
Comment 3 Serge Gautherie (:sgautherie) 2012-02-29 13:09:25 PST
(In reply to Serge Gautherie (:sgautherie) from comment #2)
> NB: I just noticed that my do_print() should probably be a print(), to match
> the rest of the file...

But maybe not, afterall.
Comment 4 Marco Bonardo [::mak] 2012-02-29 14:40:40 PST
(In reply to Serge Gautherie (:sgautherie) from comment #2)
> Created attachment 601728 [details] [diff] [review]
> (Bv1-FF) test_bookmarks_html.js: Fix nits, Add a do_print()
> 
> testImportedBookmarksToFolder() assumes the menu items come first in the
> data file.
> Is that a rule or a (bad) assumption in the test?

It is based on the test file it is importing, no magic. The exporter puts the menu first.
That said, this test sucks and is hard to maintain, though it's not something we are willing to rewrite atm, also because hsivonen is working on a js bookmarks importer in bug 482911 that also touches this test and uses it for basic testing.
One day there will be a better one, hopefully in toolkit :)
Comment 5 Marco Bonardo [::mak] 2012-02-29 14:41:10 PST
Comment on attachment 601728 [details] [diff] [review]
(Bv1-FF) test_bookmarks_html.js: Fix nits, Add a do_print()

Review of attachment 601728 [details] [diff] [review]:
-----------------------------------------------------------------

for consistency print() is better, btw I don't care that much.
Comment 6 Serge Gautherie (:sgautherie) 2012-03-01 04:21:55 PST
Created attachment 601925 [details] [diff] [review]
(Bv1a-FF) test_bookmarks_html.js: Fix nits, Add/Use do_print(), Improve documentation
[Checked in: See comment 9]

Bv1-FF, with:
*add a comment about item order.
*use do_print().
*remove a print() which in practice duplicates following do_check_eq() log.
Comment 7 Jens Hatlak (:InvisibleSmiley) 2012-03-03 09:43:31 PST
Comment on attachment 601717 [details] [diff] [review]
(Av1-SM)  Port |Bug 593566 - Bookmarks with blank name are wrongly exported (broken codepage symbols in the exported file)| to SeaMonkey

I won't be able to give this the attention it deserved any time soon, sorry.
Comment 8 Marco Bonardo [::mak] 2012-03-05 07:49:37 PST
Comment on attachment 601925 [details] [diff] [review]
(Bv1a-FF) test_bookmarks_html.js: Fix nits, Add/Use do_print(), Improve documentation
[Checked in: See comment 9]

Review of attachment 601925 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the following fixed

::: browser/components/places/tests/unit/test_bookmarks_html.js
@@ +351,1 @@
>          checkItem(group[t], container.getChild(t));

loops should always be braced, so please don't remove braces from here.
Comment 9 Serge Gautherie (:sgautherie) 2012-03-05 10:50:45 PST
Comment on attachment 601925 [details] [diff] [review]
(Bv1a-FF) test_bookmarks_html.js: Fix nits, Add/Use do_print(), Improve documentation
[Checked in: See comment 9]

https://hg.mozilla.org/mozilla-central/rev/79082093ef50
Bv1a-FF, with comment 8 suggestion(s).
Comment 10 Serge Gautherie (:sgautherie) 2012-03-05 11:13:46 PST
Created attachment 602980 [details] [diff] [review]
(Av1a-SM) Port |Bug 593566 - Bookmarks with blank name are wrongly exported (broken codepage symbols in the exported file)| to SeaMonkey
[Checked in: See comment 12]

Av1-SM, resync'ed after Bv1a-FF.
Comment 11 Serge Gautherie (:sgautherie) 2012-03-05 11:41:09 PST
(In reply to Serge Gautherie (:sgautherie) from comment #9)
> https://hg.mozilla.org/mozilla-central/rev/79082093ef50

https://tbpl.mozilla.org/php/getParsedLog.php?id=9827136&tree=Firefox&full=1
Rev3 Fedora 12 mozilla-central opt test xpcshell on 2012-03-05 10:55:14 PST for push 79082093ef50

V.Fixed wrt this FF patch (only).
Comment 12 Serge Gautherie (:sgautherie) 2012-03-18 16:35:52 PDT
Comment on attachment 602980 [details] [diff] [review]
(Av1a-SM) Port |Bug 593566 - Bookmarks with blank name are wrongly exported (broken codepage symbols in the exported file)| to SeaMonkey
[Checked in: See comment 12]

http://hg.mozilla.org/comm-central/rev/d0057605218c
Av1a-SM unbitrotted.
Comment 13 Serge Gautherie (:sgautherie) 2012-03-18 20:02:52 PDT
Comment on attachment 602980 [details] [diff] [review]
(Av1a-SM) Port |Bug 593566 - Bookmarks with blank name are wrongly exported (broken codepage symbols in the exported file)| to SeaMonkey
[Checked in: See comment 12]

[Approval Request Comment]
Needed to land bug 730849 on top of it.
Comment 14 Serge Gautherie (:sgautherie) 2012-03-19 10:31:32 PDT
(In reply to Serge Gautherie (:sgautherie) from comment #12)
> http://hg.mozilla.org/comm-central/rev/d0057605218c
> Av1a-SM unbitrotted.

Though this passes locally on old
[Mozilla/5.0 (Windows NT 5.0; rv:13.0a1) Gecko/20120201 Firefox/13.0a1 SeaMonkey/2.10a1] (nightly, 2012-02-01-00-30-09-comm-central-trunk)

it caused
{
TEST-UNEXPECTED-FAIL | xpcshell/head.js | [Exception... "Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIAnnotationService.getItemAnnotation]"  nsresult: "0x80040111 (NS_ERROR_NOT_AVAILABLE)"  location: "JS frame :: e:/builds/slave/test/build/xpcshell/tests/suite/common/places/tests/unit/test_bookmarks_html.js :: checkItem :: line 334"  data: no]
}
on our boxes.

Nonetheless, I want to land bug 730849 patch Bv1 to fully resync' this test, before addressing any issue that might remain after that.
Comment 15 Serge Gautherie (:sgautherie) 2012-03-22 16:50:43 PDT
(In reply to Serge Gautherie (:sgautherie) from comment #14)
> Nonetheless, I want to land bug 730849 patch Bv1 to fully resync' this test,
> before addressing any issue that might remain after that.

That patch did fix this (regression) error :-)

V.Fixed
Comment 16 Justin Wood (:Callek) 2012-04-15 14:06:48 PDT
Comment on attachment 602980 [details] [diff] [review]
(Av1a-SM) Port |Bug 593566 - Bookmarks with blank name are wrongly exported (broken codepage symbols in the exported file)| to SeaMonkey
[Checked in: See comment 12]

test only. a=me
Comment 17 Justin Wood (:Callek) 2012-04-16 00:35:23 PDT
Hrm fails to apply on aurora:

Justin@ORION /d/sources/comm-aurora
$ hg qpush
applying 731663-Av1a-SM_rewrite-test.diff
patching file suite/common/places/tests/unit/test_bookmarks_html.js
Hunk #1 FAILED at 9
1 out of 1 hunks FAILED -- saving rejects to file suite/common/places/tests/unit/test_bookmarks_html
.js.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh 731663-Av1a-SM_rewrite-test.diff
Comment 18 Serge Gautherie (:sgautherie) 2012-04-16 15:06:57 PDT
Created attachment 615487 [details] [diff] [review]
(Av1b-SM-a210) Port |Bug 593566 - Bookmarks with blank name are wrongly exported (broken codepage symbols in the exported file)| to SeaMonkey [Checkin: Comment 19]

(In reply to Justin Wood (:Callek) from comment #17)
> Hrm fails to apply on aurora:

That's because bug 736547 landed on c-c out of order...
Comment 19 Jens Hatlak (:InvisibleSmiley) 2012-04-17 07:55:55 PDT
Comment on attachment 615487 [details] [diff] [review]
(Av1b-SM-a210) Port |Bug 593566 - Bookmarks with blank name are wrongly exported (broken codepage symbols in the exported file)| to SeaMonkey [Checkin: Comment 19]

http://hg.mozilla.org/releases/comm-aurora/rev/aa57d84a566c
Comment 20 Serge Gautherie (:sgautherie) 2012-04-17 19:48:15 PDT
seamonkey2.10: verified, though/as it caused the same/expected/to-be-fixed test failure.
Comment 21 Serge Gautherie (:sgautherie) 2012-04-17 20:26:50 PDT
Created attachment 616001 [details] [diff] [review]
(Cv1-SM-a210) Typo fix for patch Av1b-SM-a210 [Checkin: Comment 22]
Comment 22 Jens Hatlak (:InvisibleSmiley) 2012-04-20 14:01:35 PDT
Comment on attachment 616001 [details] [diff] [review]
(Cv1-SM-a210) Typo fix for patch Av1b-SM-a210 [Checkin: Comment 22]

http://hg.mozilla.org/releases/comm-aurora/rev/6dbd42c774c3
Comment 23 Serge Gautherie (:sgautherie) 2012-04-23 19:45:39 PDT
(In reply to Serge Gautherie (:sgautherie) from comment #20)
> seamonkey2.10: verified, though/as it caused the same/expected/to-be-fixed
> test failure.

Bug 730849 comment 17: failure is now fixed.

Note You need to log in before you can comment on or make changes to this bug.