1370051 - Places - Deleting folder of livemarks - throws an errors

Status: RESOLVED WONTFIX

(Firefox :: Bookmarks & History, defect, P3)

Description

[janekptacijarabaci]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:45.9) Gecko/20100101 Goanna/3.2 Firefox/45.9 PaleMoon/27.3.0
Build ID: 20170422220104

Steps to reproduce:

- Go to a folder of livemarks
- In the context menu, select "Delete"



Actual results:

Bookmarks is deleted, but throws an errors in Browser Console:
[Exception... "Invalid livemark"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: resource://gre/components/nsLivemarkService.js :: getLivemark/< :: line 302"  data: no]



Expected results:

Bookmarks is deleted, and Browser Console is empty.

Comment 1

[janekptacijarabaci]

Patch proposal to fix issue can be:
https://github.com/MoonchildProductions/Pale-Moon/pull/1139

Comment 2

[Marco Bonardo [:mak]]

That patch doesn't look good. If the error is unexpected we should just not make it reportError, if the error is real, then we should figure out what causes it, rather than workarounding the error. Here looks like something is invoking removeLivemark on an item that is not a livemark, or not recognized by the Livemarks service, we need to figure out why that happens.

Comment 3

[janekptacijarabaci]

OK. 

In this case, I propose the following change:

Ad https://hg.mozilla.org/mozilla-central/file/291fe1a6a375/toolkit/components/places/PlacesUtils.jsm#l3307

From:

for (let i = 0; i < contents.childCount; ++i) {
  let txn = new PlacesRemoveItemTransaction(contents.getChild(i).itemId);
  transactions.push(txn);
}

To:

for (let i = 0; i < contents.childCount; ++i) {
  let childId = contents.getChild(i).itemId;
  if (!PlacesUtils.annotations.itemHasAnnotation(childId,
                                                 PlacesUtils.LMANNO_FEEDURI)) {
    let txn = new PlacesRemoveItemTransaction(childId);
    transactions.push(txn);
  }
}

Comment 4

[Marco Bonardo [:mak]]

(In reply to janekptacijarabaci from comment #3)
> for (let i = 0; i < contents.childCount; ++i) {
>   let childId = contents.getChild(i).itemId;
>   if (!PlacesUtils.annotations.itemHasAnnotation(childId,
>                                                 
> PlacesUtils.LMANNO_FEEDURI)) {

This is still doing main-thread IO for every item to check if it has an annotation. 
The view should already know if it's a livemark or not, IIRC we keep some sort of local property to track those.

Comment 5

[Lorienna]

* Regressed by:

Bug 969318 - "A promise chain failed to handle a rejection" from the livemarks service (deprecate passing a callback)

Changeset 173276 - Mute "A promise chain failed to handle a rejection" error from the livemarks service. r=mano: 
https://hg.mozilla.org/mozilla-central/rev/60c1af78091a

Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=44ae8462d6ab&tochange=46041cc216fd

First bad: 30.0a1 (2014-03-13)
Built from https://hg.mozilla.org/mozilla-central/rev/46041cc216fd

Last good: 30.0a1 (2014-03-12)
Built from https://hg.mozilla.org/mozilla-central/rev/44ae8462d6ab


* The earliest error flavor:

[Exception... "Illegal value"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: resource://gre/components/nsLivemarkService.js :: LS_getLivemark/< :: line 397"  data: no]
Source File: resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js
Line: 870

Comment 6

[Marco Bonardo [:mak]]

Wontfixing because live bookmarks are going away - See Bug 1477667.