Last Comment Bug 663763 - Provide helper function for opening the library in tests
: Provide helper function for opening the library in tests
Product: Firefox
Classification: Client Software
Component: Bookmarks & History (show other bugs)
: Trunk
: All All
-- normal (vote)
: Firefox 7
Assigned To: Dão Gottwald [:dao]
: Marco Bonardo [::mak]
Depends on:
Blocks: 725549
  Show dependency treegraph
Reported: 2011-06-13 00:44 PDT by Dão Gottwald [:dao]
Modified: 2012-03-07 05:03 PST (History)
1 user (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (27.09 KB, patch)
2011-06-13 00:44 PDT, Dão Gottwald [:dao]
mak77: review+
Details | Diff | Splinter Review

Description User image Dão Gottwald [:dao] 2011-06-13 00:44:25 PDT
Created attachment 538825 [details] [diff] [review]
Comment 1 User image Marco Bonardo [::mak] 2011-06-17 06:12:28 PDT
any reason to change reviewer here? I can do this, I'd rather not disturb sdwilsh for this stuff.
Comment 2 User image Dão Gottwald [:dao] 2011-06-17 06:15:42 PDT
Feel free to steal from Shawn. I just had the impression that you're busy with other stuff, since this waited a few days even though I expected this to be a very quick review.
Comment 3 User image Marco Bonardo [::mak] 2011-06-17 06:27:05 PDT
Comment on attachment 538825 [details] [diff] [review]

Review of attachment 538825 [details] [diff] [review]:

Sorry sometimes context switching is expensive, I'm running through reviews queue now.

So, I like this, the only doubt I still have is whether we may need an additional executeSoon due to the fact some of the Library trees stuff is inited lazily (especially the details pane overlay). I know waitForFocus does that once and I assume if you ran this through try so it may be enough as it is.
But as a protection vs randomness, I'd prefer it this may land first on Places and bake a couple days. I can push it if you don't want to clone the branch.
Comment 4 User image Dão Gottwald [:dao] 2011-06-17 06:32:15 PDT
Feel free to land this on places. That tree appears to be in a bad shape, though...
Comment 5 User image Marco Bonardo [::mak] 2011-06-17 06:37:25 PDT
Yes, I was working on that to be able to merge.
I have a local patch that should fix all livemarks tests random failures, I'll need a review on that but then it should go green. Since these changes are on b-c tests and other failures are on chrome-tests it should be fine.

Will land in the next minutes along with your other bug 663630.
Comment 6 User image Marco Bonardo [::mak] 2011-06-17 07:41:17 PDT
Sorry, it took a while to figure out how to make Mercurial correctly handle your name :)
Comment 7 User image Marco Bonardo [::mak] 2011-06-18 04:44:27 PDT

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