Closed
Bug 1353783
Opened 7 years ago
Closed 7 years ago
updatePlaces handleCompletion doesn't return embed visits in its updatedCount argument
Categories
(Toolkit :: Places, defect, P2)
Toolkit
Places
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: mak, Assigned: mak)
References
Details
(Whiteboard: [fxsearch])
Attachments
(1 file)
See https://bugzilla.mozilla.org/show_bug.cgi?id=1343182#c28 UpdatePlaces has this updatedCount that returns in handleCompletion, this doesn't account for embed visits, so it looks like no visit was added, when instead some were added. And embed visits should not notify onError and onResult if those are ignored by the callback.
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8855290 [details] Bug 1353783 - updatePlaces doesn't correctly notify and count embed visits. https://reviewboard.mozilla.org/r/127166/#review129936 Code LGTM but I don't know about all the android stuff... ::: toolkit/components/places/tests/favicons/xpcshell.ini (Diff revision 1) > -# Bug 676989: test fails consistently on Android > -fail-if = os == "android" As noted on IRC, not sure what these removals are about. Are these just dead test annotations?
Attachment #8855290 -
Flags: review?(gijskruitbosch+bugs) → review+
Assignee | ||
Comment 3•7 years ago
|
||
(In reply to :Gijs from comment #2) > ::: toolkit/components/places/tests/favicons/xpcshell.ini > (Diff revision 1) > > -# Bug 676989: test fails consistently on Android > > -fail-if = os == "android" > > As noted on IRC, not sure what these removals are about. Are these just dead > test annotations? My fault, I should have annotated that in the commit message. Basically we can remove those for 2 reasons: 1. These are from xul fennec, the current Firefox for Android doesn't use Places at all. 2. in any case the same is repeated in the head part of the ini file, so no reason to have single annotations.
Assignee | ||
Comment 4•7 years ago
|
||
(fwiw, the current Android build should not even build the places/ folder at all)
Comment hidden (mozreview-request) |
Pushed by mak77@bonardo.net: https://hg.mozilla.org/integration/autoland/rev/64f302609496 updatePlaces doesn't correctly notify and count embed visits. r=Gijs
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/64f302609496
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment 8•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/64f302609496
You need to log in
before you can comment on or make changes to this bug.
Description
•