Bug 620198 affects our new Places-based Bookmarks Manager, too (a single-bookmark test case failed for me, but one with three bookmarks worked).
Ignoring tests, FF's fix essentially consists of two parts:
1. changes to the "batching" function. Looks like a rip-off of what we came up with during our review process in bug 613034, so AFAICS nothing to do here.
2. changes to the "setTree" function. I think this is all we need.
Note that like in bug 613034 we probably need to fix treeView.js in two places, suite/common/places/ and suite/common/history/.
Created attachment 505451 [details] [diff] [review]
patch [Checkin: comment 7]
Argh, I filed bug 627416 for this and two other small things we should port from the same push, I hoped someone would do them all at once so we don't need a lot of very small patches...
(In reply to comment #2)
> Argh, I filed bug 627416 for this and two other small things we should port
> from the same push, I hoped someone would do them all at once so we don't need
> a lot of very small patches...
1. I did the analysis (that the underlying issue also applies to SM), filed this bug, linked it in the appropriate places, created the patch and added it here, all before you filed that bug. The reason was that the bug this change is based on has been ported by me so I knew the code.
2. There has been a lot of back-and-forth bugspam simply because both you and Serge didn't check your FF2SM bugmail before you added depends/blocks bug references. I did.
3. I think that one issue per bug is generally better to track, and fosters individual fixing and landing. From past experience I've been under the impression that this is your view, too. I didn't follow that rule myself when porting Sync, but there the consolidation was far greater (more than two dozen base bugs) and in the end I still had to move the dialogs part to a new bug to maintain a sense of sanity.
That said, if anyone wants to do all the work in the bug you filed, obsoleting this one, fine. That won't be me then, though.
Whatever, shoot me down, hang me up, what you like. Should I mark the other bug as INVALID?
(In reply to comment #4)
> Whatever, shoot me down, hang me up, what you like. Should I mark the other bug
> as INVALID?
No, it is valid. You explained your reasoning and it makes sense to me. That doesn't mean that I'll have to be the one who takes that bug, though. Anyone can, and if someone does, I'll happily close this as a dupe of yours. Or if the reviewer here tells me to do so. Or you do. Until then I'd rather fix at least one thing (that I already analyzed) than nothing.
I think the three of us just wanted to be helpful. It just occurred to me that people address issues differently, and that's OK. I just wished that people took greater care at times, even when it comes to things that are actually not so important details, as part of being polite. But then that's just me; YMMV. Maybe I should have voiced that differently, or kept it to myself altogether as ever so often. Anyway, this is probably better taken to the newsgroups if anyone cares. Sorry for the bugspam.
's OK, I may have phrased things suboptimally myself, and I'm happy about any work that gets done for sure! ;-)
That said, I didn't have any intentions of doing any of this myself, just wanted it to be filed ASAP.
Comment on attachment 505451 [details] [diff] [review]
patch [Checkin: comment 7]
(In reply to comment #3)
> 2. There has been a lot of back-and-forth bugspam simply because both you and
> Serge didn't check your FF2SM bugmail before you added depends/blocks bug
> references. I did.
Just ftr, I think I did check but those emails were somehow delayed for me.
Anyway, I concur we were just 3 people trying to do the same thing in 3 different ways "at the same time".
Sorry about the spam when I noticed and fixed the situation.