1681818 - "Restore Default Search Engines" button removes tabs, bookmark and history's search shortcuts from the list

Status: VERIFIED FIXED

(Firefox :: Search, defect, P1)

Description

[Florencia Di Ciocco, QA]

Attachment: video of bug

[Sugested Severity:]
S3

[Description:]
when I remove search shortcuts in a particular order and then press "Restore Default Search Engines", Bookmarks, Tabs, and History are removed forever.

[Steps:]

1. Open Firefox with a new profile.
2. Go to about:preferences#search
3. Remove two search shortcuts immediately above Bookmarks
4. Remove the rest of bookmarks from top to bottom. It's important to press remove twice or thrice after removing all possible search engines.
5. Hit Restore Default Search Engines

[Actual result:]
Search engines are restored but bookmarks, tabs, and history are lost forever

[Expected result:]
Search engines are restored and bookmarks, tabs, and history are kept

Comment 1

[Mark Banner (:standard8)]

Drew, could you have a look and prioritise this?

Comment 2

[Marco Bonardo [:mak]]

If I close and reopen preferences, the buttons appear again, so it's not a permanent loss.

Comment 3

[Florencia Di Ciocco, QA]

That's true. But I have to click on "restore default search engines" again to recover the search engine that wasn't restored before. Because with the str from above you lose the bookmarks, tabs, and history, AND one of the default search engines.

Comment 4

[Drew Willcoxon :adw]

Yeah that's not good.

Comment 5

[Drew Willcoxon :adw]

Attachment: Bug 1681818 - In the search preferences UI, fix tree view problems that arise when engines are removed and restored due to the local shortcuts being present.

The problem is that a few sites assume that lastIndex is the index of the last
engine in the tree, but since bug 1657790 landed, lastIndex is now the index
of the last local shortcut.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/caf10909f9c6
In the search preferences UI, fix tree view problems that arise when engines are removed and restored due to the local shortcuts being present. r=mak

Comment 7

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/caf10909f9c6

Comment 8

[Florencia Di Ciocco, QA]

Hi, I verified this on 86.0a1 (2020-12-23) (64-bit).

Regards, Flor.

Comment 9

[Drew Willcoxon :adw]

[Tracking Requested - why for this release]:

This bug regresses the behavior of the "Restore Default Search Engines" and "Remove" buttons in the search preferences UI in certain conditions, and the regression is due to bug 1657790, so I'll mark it as such. The regression is present on 85, and the ability to remove and restore search engines is important, so I think we should track this. As Marco points out, the buggy behavior goes away once you close and reopen the preferences, but that's not obvious. I'll request uplift next.

Comment 10

[Drew Willcoxon :adw]

Attachment: Beta 85 patch

Approval Request Comment
[Feature/Bug causing the regression]:
Bug 1657790

[User impact if declined]:
In some cases when users remove search engines in the preferences UI, the "Restore Default Search Engines" and "Remove" buttons will not work correctly, and the list of engines will not be accurate.

[Needs manual test from QE? If yes, steps to reproduce]:
Already verified (comment 8)

[Is the change risky?]:
Low risk

[Why is the change risky/not risky?]:
The patch is small and only updates one variable used to track the index of the last engine in the list. It has a test and has been verified on Nightly, and I've locally verified this Beta patch.

Comment 11

[Julien Cristau [:jcristau]]

Comment on attachment 9194552 [details] [diff] [review]
Beta 85 patch

approved for 85.0b5

Comment 12

[Release mgmt bot [:sylvestre / :calixte / :marco for bugbug]]

Changing the priority to p1 as the bug is tracked by a release manager for the current beta.
See What Do You Triage for more information

Comment 13

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/cec4063549ff

Comment 14

[Florencia Di Ciocco, QA]

Hi,

I just verified this on Win 10 on FF beta 85.0b5 (64-bit). It works ok, the bug is fixed.

Regards, Flor.