I actually comment on those in bug 583386. Please pay more attention to review comments.

(In reply to comment #0) > I actually comment on those in bug 583386. s/comment/commented/

historyUndoPopup leads with two clones.

Ok, I have fixed historyUndoPopup, I'll look for more and fix those as well.

Tried to get all the ids more or less in shape. Since I don't have a build environment set up on Windows, I've submitted this to the try server. Requesting review early as there's some time pressure (bug 583386 comment 131).

(In reply to comment #4) > Since I don't have a build > environment set up on Windows, I've submitted this to the try server. (I did however verify that the browser-places.js changes aren't completely wrong by building locally.)

added missing split-menuitem-item classes

Wouldn't the missing split-menuitem-item classes break styling significantly? Reviewing this quickly is kind of hell. I take it all of the items you're changing have been recently added, and don't have any other references (beyond the ones you're updating?).

I've applied your patch and the undo closed tab/window menus work fine. The 'Subscribe to this page' no longer works because the id was changed.

(In reply to comment #8) > Wouldn't the missing split-menuitem-item classes break styling significantly? They do on XP (all) and Win7 (classic). > Reviewing this quickly is kind of hell. I take it all of the items you're > changing have been recently added, and don't have any other references (beyond > the ones you're updating?). Yeah, except for those that had duplicates that are being referenced, like historyUndoPopup. (In reply to comment #10) > I've applied your patch and the undo closed tab/window menus work fine. The > 'Subscribe to this page' no longer works because the id was changed. Another pre-existing and referenced id, which means it's impossible for both "Subscribe to This Page..." instances to work right now... FeedHandler.updateFeed in browser.js needs an update. If this is the only remaining issue, I'd spin it off to a separate bug.

(In reply to comment #11) > > I've applied your patch and the undo closed tab/window menus work fine. The > > 'Subscribe to this page' no longer works because the id was changed. > > Another pre-existing and referenced id, which means it's impossible for both > "Subscribe to This Page..." instances to work right now... > FeedHandler.updateFeed in browser.js needs an update. If this is the only > remaining issue, I'd spin it off to a separate bug. filed bug 589212

(In reply to comment #4) > Created attachment 467769 [details] [diff] [review] > WIP patch > > Tried to get all the ids more or less in shape. Since I don't have a build > environment set up on Windows, I've submitted this to the try server. http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/dgottwald@mozilla.com-2b9dbb3300d6

(In reply to comment #13) > (In reply to comment #4) > > Created attachment 467769 [details] [diff] [review] [details] > > WIP patch > > > > Tried to get all the ids more or less in shape. Since I don't have a build > > environment set up on Windows, I've submitted this to the try server. > > http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/dgottwald@mozilla.com-2b9dbb3300d6 Haven't encountered anything unexpected so far.

minor last-minute cleanup

Looks like I won't be able to land this right now, but I'll try to make sure it's in the next nightly.

http://hg.mozilla.org/mozilla-central/rev/8280bf7b49aa http://hg.mozilla.org/mozilla-central/rev/8f2e9f567713