1434261 - Unable to add/remove bookmarks after deleting a folder from the star UI

Status: RESOLVED FIXED

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

Comment 1

[Alice0775 White]

Reproducible: always

Steps to reproduce:
1. Click Start icon
2. Expand folder tree (Click "Show all the bookmarks folders" doropdown button)
3. Select a sub folder
4. Press [Del] key
5. Click "Remove Bookmark"

6. Try to add/delete/move any bookmarks from Star/Sidebar/Library/toolbar/menubar

Actual Results:
Nothing happens.
The functions are no longer working until restart browser.

Expected Results:
Should be working properly

Comment 2

[Marco Bonardo [:mak]]

Is anything printed to the Browser Console?

Comment 3

[Alice0775 White]

(In reply to Marco Bonardo [::mak] from comment #2)
> Is anything printed to the Browser Console?


No error in Browser Console.



Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=0da00124af43d44fed96133300ba5e32b0d821a8&tochange=0a4690dfd7b383e2f732210cf8906ce51a5b2433

Triggered by: 0a4690dfd7b3	Mark Banner — Bug 1071513 - Enable async PlacesTransactions for nightly builds. r=mak


setting browser.places.useAsyncTransactions = false fixes the problem on Firefox58.(Although, folders are deleted and "Remove Bookmark" button becomes disable)

Comment 4

[Mark Banner (:standard8)]

I'm investigating this at the moment - I can reproduce it.

My initial instinct is that we could/should disable delete in the folder area - there's no real need, unless you accidentally create a folder which is less likely, and if you do accidentally delete a folder, you might be loosing more than you wanted.

However, I would like to understand what's going on, I think this is something in the transaction undo/redo handler but it is kinda strange.

Comment 5

[Mark Banner (:standard8)]

I think what is going on here, is that the star UI/properties dialog has a batching mechanism which automatically batches all transactions. However, the places tree uses the separate controller which also has its own batching for removing items.

The combination of the two, means the delete can't be enacted straight away, as the properties dialog is holding the batch until it completes. When it does complete, the delete is enacted, but due to the remove bookmark, the actions can't be undone and the transaction system effectively blows up, and causes most bookmark actions to stop working.

Comment 6

[Mark Banner (:standard8)]

Attachment: Bug 1434261 - Disable user actions on the places trees not in the library window to avoid accidential/unintentional actions.

Review commit: https://reviewboard.mozilla.org/r/222724/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/222724/

Comment 7

[Marco Bonardo [:mak]]

Comment on attachment 8953492 [details]
Bug 1434261 - Disable user actions on the places trees not in the library window to avoid accidential/unintentional actions.

https://reviewboard.mozilla.org/r/222724/#review229522

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/77d5fc984c50
Disable user actions on the places trees not in the library window to avoid accidential/unintentional actions. r=mak

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/77d5fc984c50

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

I'd like to let this fix ride the trains with 60 unless someone feels strongly about fixing it in 59.