http://searchfox.org/mozilla-central/source/browser/modules/WindowsJumpLists.jsm#464 Should be replaced with the new PlacesUtils.history.remove API that is promise based, in this case we should add Cu to the top and .catch(Cu.reportError).

Hi I want to dive into it.Any documentation for the PlacesUtils.history.remove API, to implement it in WindowsJumpLists.jsm#464

(In reply to Hemant Singh Patwal [:hhhh1612] from comment #1) > I want to dive into it.Any documentation for the PlacesUtils.history.remove > API, to implement it in WindowsJumpLists.jsm#464 PlacesUtils.history is in History,jsm, specifically: https://dxr.mozilla.org/mozilla-central/rev/96b36c5f527dd42e680a230839519eee1fc2c9f3/toolkit/components/places/History.jsm#255

Hi! Hemant, are you still interested in this bug? I'm happy to take it if you are busy.

Sure, due to exams I couldn't look into it.Happy you are taking it.

Comment on attachment 8873941 [details] Bug 1364337 - Remove a deprecated RemovePages call from WindowsJumpLists.jsm. https://reviewboard.mozilla.org/r/145318/#review149294 ::: browser/modules/WindowsJumpLists.jsm:464 (Diff revision 1) > let uriSpec = oldItem.app.getParameter(0); > URIsToRemove.push(NetUtil.newURI(uriSpec)); > } catch (err) { } > } > } > if (URIsToRemove.length > 0) { This check is also made inside the remove(), but there, if length == 0, it throws an error. Should I remove this if ?

Comment on attachment 8873941 [details] Bug 1364337 - Remove a deprecated RemovePages call from WindowsJumpLists.jsm. https://reviewboard.mozilla.org/r/145320/#review149690 ::: browser/modules/WindowsJumpLists.jsm:6 (Diff revision 1) > /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */ > /* This Source Code Form is subject to the terms of the Mozilla Public > * License, v. 2.0. If a copy of the MPL was not distributed with this > * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ > > +Components.utils.import("resource://gre/modules/PlacesUtils.jsm"); Please use XPCOMUtils.defineLazyModuleGetter, we don't want to pay the price until it's needed. ::: browser/modules/WindowsJumpLists.jsm:464 (Diff revision 1) > let uriSpec = oldItem.app.getParameter(0); > URIsToRemove.push(NetUtil.newURI(uriSpec)); > } catch (err) { } > } > } > if (URIsToRemove.length > 0) { keeping the if is fine, remove() throws if the array is empty because it assumes the consumer did some mistake trying to remove an empty array. ::: browser/modules/WindowsJumpLists.jsm:465 (Diff revision 1) > URIsToRemove.push(NetUtil.newURI(uriSpec)); > } catch (err) { } > } > } > if (URIsToRemove.length > 0) { > - PlacesUtils.bhistory.removePages(URIsToRemove, URIsToRemove.length, true); > + PlacesUtils.history.remove(URIsToRemove).catch(Cu.reportError); Cu.reportError won't work because you didn't define Cu at the top of the file. While there you could probably replace the current code with: const { classes: Cc, interfaces: Ci, results: Cr, utils: Cu } = Components;

Comment on attachment 8873941 [details] Bug 1364337 - Remove a deprecated RemovePages call from WindowsJumpLists.jsm. https://reviewboard.mozilla.org/r/145320/#review150048 r=me with a couple fixes. once you'll attach an updated patch I'll push it to Try to quickly check tests are fine (I'm not sure whether this is tested honestly, but it won't hurt) ::: browser/modules/WindowsJumpLists.jsm:53 (Diff revision 2) > -}); > - > XPCOMUtils.defineLazyGetter(this, "NetUtil", function() { > - Components.utils.import("resource://gre/modules/NetUtil.jsm"); > + Cu.import("resource://gre/modules/NetUtil.jsm"); > return NetUtil; > }); please, while here remove this "NetUtil" getter completely, we'll fix the only consumer down here... ::: browser/modules/WindowsJumpLists.jsm:452 (Diff revision 2) > while (e.hasMoreElements()) { > let oldItem = e.getNext().QueryInterface(Ci.nsIJumpListShortcut); > if (oldItem) { > try { // in case we get a bad uri > let uriSpec = oldItem.app.getParameter(0); > URIsToRemove.push(NetUtil.newURI(uriSpec)); Just use Services.io.newURI here instead of NetUtil.newURI

there's a small whitespace nit, I'll fix it before pushing.

Comment on attachment 8875401 [details] Bug 1364337 - Remove a deprecated RemovePages call from WindowsJumpLists.jsm. https://reviewboard.mozilla.org/r/146844/#review150914

Pushed by mak77@bonardo.net: https://hg.mozilla.org/integration/autoland/rev/9721edbfbba3 Remove a deprecated RemovePages call from WindowsJumpLists.jsm. r=mak