762824 - Move Bookmark stopped working

Status: RESOLVED FIXED

(SeaMonkey :: Bookmarks & History, defect)

Description

[Jens Hatlak (:InvisibleSmiley)]

Move Bookmark (from the Bookmarks Manager Tools menu) stopped working some time ago already but no-one noticed (well, I did, but thought it was due to one of the many add-ons I have installed). The effect is that clicking the OK button does nothing. Nothing on the Error Console either when that happens.

Unfortunately this bug made it into our latest release, 2.10 (which notably does not contain the fix for bug 730837 yet). Last known good is 2.9.1.

STR:
1. Open Bookmarks Manager
2. Select some bookmark
3. Open Tools/Move...
4. Select some folder (other than the one the selected bookmark resides in)
5. Click OK

Expected result: Dialog closes, bookmark is moved.

Actual result: Nothing (dialog still open, bookmark not moved).

I don't know by heart how the Places stuff is connected; maybe this is fallout from some of the reorganizations we did.

Comment 1

[Jens Hatlak (:InvisibleSmiley)]

Actually it's the Edit menu.

FWIW: Works with latest FF nightly.

Comment 2

[Jens Hatlak (:InvisibleSmiley)]

It looks to me like the following line was ported incorrectly in bug 732027:

let txn = PlacesMoveItemTransaction(this._nodes[i].itemId,

should be

let txn = new PlacesMoveItemTransaction(this._nodes[i].itemId,

Cf.

http://hg.mozilla.org/mozilla-central/diff/322e727576c8/browser/components/places/content/moveBookmarks.js

Will try a local patch now.

Comment 3

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch [Checkin: Comment 7]

[Approval Request Comment]
Regression caused by (bug #): bug 732027
User impact if declined: Move Bookmarks broken
Testing completed (on m-c, etc.): confirmed locally
Risk to taking this patch (and alternatives if risky): minimal
String changes made by this patch: none

Comment 4

[Frank Wein [:mcsmurf]]

I'll never ever touch bookmarks code again.

btw: Moving via drag&drop works (which is what I use all the time, I did not even know we have a move dialog).

Comment 5

[Philip Chee]

> I'll never ever touch bookmarks code again.
Never is a long time. How about next Tuesday?

Comment 6

[Philip Chee]

Comment on attachment 631325 [details] [diff] [review]
patch [Checkin: Comment 7]

r=me

Comment 7

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 631325 [details] [diff] [review]
patch [Checkin: Comment 7]

http://hg.mozilla.org/comm-central/rev/335ef42ee9ac
http://hg.mozilla.org/releases/comm-aurora/rev/89e29f9ab6fb
http://hg.mozilla.org/releases/comm-beta/rev/20a9998d99a7
http://hg.mozilla.org/releases/comm-release/rev/973c61c27265

(In reply to Frank Wein [:mcsmurf] from comment #4)
> I'll never ever touch bookmarks code again.

Heh. Understandable feeling, but no-one's blaming you really. I guess we've all learned our lesson from this "landing untested code changes on branches" debacle. At least we have a bug to point at for reference now. ;-)

Comment 8

[Jens Hatlak (:InvisibleSmiley)]

Flags/releases explanation: This bug is fixed in
* the next SM trunk nightly 
* the next SM 2.12a2 (Aurora) nightly
* SM 2.11b2 (2.11b1 has already been tagged)
* 2.10.1 iff it will ever be released and will include this fix.