Closed Bug 1410928 Opened 2 years ago Closed 2 years ago

If you save a bookmark (by accident) as --- http://www.example.com>/ --- (with the added >) .. you cannot anymore edit/delete the bookmark

Categories

(Toolkit :: Places, defect, P2)

57 Branch
defect

Tracking

()

RESOLVED DUPLICATE of bug 1401401

People

(Reporter: simonandric5, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [fxsearch])

Attachments

(1 file)

Attached image Untitled.jpg
User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20171019140425

Steps to reproduce:

I copied a URL from a html file and pasted it in the /Show All bokomarks+right click+add new bookmark/  Then I clicked OK. And tht was that. It happened in version ~ 52 (i think --- around 6 months ago). Now i use version 58 (nightly) but have snyc enabled across all three distributions (nightly, dev and release) in win and linux.



Actual results:

Since then --- i cannot edit, copy, delete, cut or do anything with this bookmark ( i can still do everything with other bookmarks) --and the folders/subfolders in which this bookmark is located -- i can rename folder but not delete it.

What is worse --- my firefox sync stopped working because of that... i have firefox sync enabled on three different desktop computers and now i see that bookmarks which i saved in one computer stayed only there -- didnt move everywhere like it should.


Expected results:

Even with this character in URL (although is invalid) the bookmark itself still should be editable/deletable etc...

ps. to reproduce you can do the same, but warning.... once it is in, it doesnt want to come out again... so do it in an environment where there is no that many bookmarks (i have more than 10.000)
Component: Untriaged → Places
Product: Firefox → Toolkit
I've just confirmed this locally. FF 51.0a1 (20160801074053) allows you to insert a bad bookmark into the database.

The latest build doesn't allow inserting it, but as the reporter describes, you also can't do anything with it. There's various errors on the console.

When trying to display the bookmark in the library so that you can change its properties:

JavaScript error: resource://gre/modules/NetUtil.jsm, line 209: NS_ERROR_MALFORMED_URI: Component returned failure code: 0x804b000a (NS_ERROR_MALFORMED_URI) [nsIIOService2.newURI]

When trying to delete the bookmark:

console.error: 
  Message: Error: Failed to get info for the specified item (guid: fHVgS7p29Oo2): Error: Failed to fetch the data for the root item [Exception... "Component returned failure code: 0x804b000a (NS_ERROR_MALFORMED_URI) [nsIIOService2.newURI]"  nsresult: "0x804b000a (NS_ERROR_MALFORMED_URI)"  location: "JS frame :: resource://gre/modules/NetUtil.jsm :: NetUtil_newURI :: line 209"  data: no]
  Stack:
    promiseBookmarksTree@resource://gre/modules/PlacesTransactions.jsm:1491:15
async*execute@resource://gre/modules/PlacesTransactions.jsm:1498:31
Async*transact/promise<@resource://gre/modules/PlacesTransactions.jsm:546:26
async*enqueue/promise<@resource://gre/modules/PlacesTransactions.jsm:472:58
promise callback*enqueue@resource://gre/modules/PlacesTransactions.jsm:472:19
transact@resource://gre/modules/PlacesTransactions.jsm:543:19
transact@resource://gre/modules/PlacesTransactions.jsm:231:16
batch/<@resource://gre/modules/PlacesTransactions.jsm:332:19
async*batch/<@resource://gre/modules/PlacesTransactions.jsm:566:20
async*enqueue/promise<@resource://gre/modules/PlacesTransactions.jsm:472:58
promise callback*enqueue@resource://gre/modules/PlacesTransactions.jsm:472:19
batch@resource://gre/modules/PlacesTransactions.jsm:561:12
batch@resource://gre/modules/PlacesTransactions.jsm:329:14
_removeRowsFromBookmarks/<@chrome://browser/content/places/controller.js:972:17
async*batchUpdatesForNode@resource:///modules/PlacesUIUtils.jsm:1509:13
async*_removeRowsFromBookmarks@chrome://browser/content/places/controller.js:971:15
async*remove@chrome://browser/content/places/controller.js:1045:15
async*PC_doCommand@chrome://browser/content/places/controller.js:245:7
goDoPlacesCommand@chrome://browser/content/places/controller.js:1829:5
oncommand@chrome://browser/content/places/places.xul:1:1
Whiteboard: [fxsearch]
The promiseBookmarksTree code currently say we'll let NetUtil throw and the item will be skipped:

http://searchfox.org/mozilla-central/rev/1285ae3e810e2dbf2652c49ee539e3199fcfe820/toolkit/components/places/PlacesUtils.jsm#1776

Note sure that's what we really want to do in this case...but not sure what impact it would have elsewhere.

I also took a brief look at allowing editing of the bookmark, but this also ends up in similar issues where Bookmarks.jsm can't understand what is in the database and throws as a result of the bad URI.

The only other though I had was a background task to clean up invalid URIs (i.e. probably remove them).

I'm not quite sure what is best/simplest to do here.
(In reply to Mark Banner (:standard8) from comment #2)
> The only other though I had was a background task to clean up invalid URIs
> (i.e. probably remove them).

We have various other bugs about similar issues, and it's likely a good idea, though, it may be expensive so we need a way to do that in chunks. I suppose we could use the place id to chunk and store it somewhere (a temp pref) then do the cleanup on idle timers. This may require a new approach to PlacesDBUtils to do cleanups in small chunks every day, vs doing a large weekly cleanup.
Blocks: 1410877
Priority: -- → P2
No longer blocks: 1410877
Blocks: 1410877
Status: UNCONFIRMED → NEW
Ever confirmed: true
Since we don't allow anymore to create invalid bookmarks, what's left is being unable to get rid of them.
So this is pretty much the same as bug 1401401.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1401401
You need to log in before you can comment on or make changes to this bug.