Closed
Bug 748562
Opened 10 years ago
Closed 10 years ago
Kill AddPageWithDetails, in Sync
Categories
(Firefox :: Sync, defect)
Firefox
Sync
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: Paolo, Assigned: Paolo)
References
Details
Attachments
(1 file, 2 obsolete files)
10.08 KB,
patch
|
Details | Diff | Splinter Review |
We're removing AddPageWithDetails in bug 739213.
Assignee | ||
Comment 1•10 years ago
|
||
Extracted from the patch in bug 739213.
Attachment #618059 -
Flags: review?(rnewman)
Assignee | ||
Comment 2•10 years ago
|
||
This also removes some code that is not required anymore.
Attachment #618059 -
Attachment is obsolete: true
Attachment #618059 -
Flags: review?(rnewman)
Attachment #618072 -
Flags: review?(rnewman)
Comment 3•10 years ago
|
||
Comment on attachment 618072 [details] [diff] [review] Replace calls to AddPageWithDetails Review of attachment 618072 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, thanks! r+ with nits. ::: services/sync/tests/unit/test_history_store.js @@ +91,5 @@ > + transitionType: Ci.nsINavHistoryService.TRANSITION_LINK > + }] > + }; > + PlacesUtils.asyncHistory.updatePlaces(place, { > + handleError: function () do_throw("Unexpected error in adding visit."), Please use full {} function style to avoid tripping up Emacs users. (I know, I know...) Also, please name all functions: handleError: function handleError() { do_throw(...); }, @@ +92,5 @@ > + }] > + }; > + PlacesUtils.asyncHistory.updatePlaces(place, { > + handleError: function () do_throw("Unexpected error in adding visit."), > + handleResult: function () { }, No space between {}, name function. @@ +93,5 @@ > + }; > + PlacesUtils.asyncHistory.updatePlaces(place, { > + handleError: function () do_throw("Unexpected error in adding visit."), > + handleResult: function () { }, > + handleCompletion: function () afterAddVisit() Just handleCompletion: afterAddVisit @@ +99,2 @@ > > + function afterAddVisit() { Perhaps 'onVisitAdded'? ::: services/sync/tests/unit/test_places_guid_downgrade.js @@ +104,5 @@ > + ]; > + PlacesUtils.asyncHistory.updatePlaces(places, { > + handleError: function () do_throw("Unexpected error in adding visit."), > + handleResult: function () { }, > + handleCompletion: function () afterAddVisit() Same comments for these three lines. @@ +201,5 @@ > > function run_test() { > setPlacesDatabase("places_v10_from_v11.sqlite"); > > _("Verify initial setup: v11 database is available"); Please move this comment into test_initial_state.
Attachment #618072 -
Flags: review?(rnewman) → review+
Assignee | ||
Comment 4•10 years ago
|
||
Attachment #618072 -
Attachment is obsolete: true
Assignee | ||
Comment 5•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d030e76e9ed3
Target Milestone: --- → mozilla15
Comment 6•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d030e76e9ed3
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•4 years ago
|
Component: Firefox Sync: Backend → Sync
Product: Cloud Services → Firefox
You need to log in
before you can comment on or make changes to this bug.
Description
•