1572329 - Show History dropdown from Address Bar in Browsing History mode does not allow the deletion of the top item

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect, P3)

Description

[adamtokarsky]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

Using Firefox Quantum 68.0.1 (64-bit), with the setting in Options: Privacy & Security: Address Bar set to Browsing History only, click the Show History down-arrow button in the Address Bar. Down arrow key to highlight the top suggested entry. Press shift+delete. Press it a second time just to make sure you didn't hit the wrong keys.

Actual results:

Top entry is not deleted from list.

Expected results:

Top entry should be deleted from the list.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Drew Willcoxon :adw]

Thanks for reporting this. Happens in Nightly too. This error is logged:

Failed to find the selected result in the results
    _handleDeleteEntry resource:///modules/UrlbarController.jsm:563
    handleKeyNavigation resource:///modules/UrlbarController.jsm:370
    _on_keydown resource:///modules/UrlbarInput.jsm:1727
    maybeDeferEvent resource:///modules/UrlbarEventBufferer.jsm:157
    _on_keydown resource:///modules/UrlbarInput.jsm:1726
    handleEvent resource:///modules/UrlbarInput.jsm:353

Comment 4

[Drew Willcoxon :adw]

I can't reproduce this on 77 or the current Nightly. The UX has changed since this was filed, but you can get the list of frecent history by typing ^ or space in the urlbar. Shift-delete on the first history result successfully deletes it. You can't delete the search result but that's by design.

If you can still reproduce this, please comment here, thanks.

Comment 5

[adamtokarsky]

Bug still present in 77.0.1 (64-bit), running Windows 10, though the method no longer involves the "Show History" button--instead, clicking in empty space of the URL bar is sufficient to show the same menu.

Comment 6

[Drew Willcoxon :adw]

That menu is a little different. Previously it showed your most frecent sites from history, but now it shows your "top sites" from about:newtab. Those sites can't be deleted by Shift-Delete. Off hand I'm not sure whether that's by design or an oversight, but it's not the same bug as the original one, which happened due to a coding error.

Comment 7

[adamtokarsky]

The sites from that menu can, in fact, be removed from the list with shift+delete.

Except for the top one.

I can resubmit as a new bug if that's prudent.

Comment 8

[Drew Willcoxon :adw]

We must not be talking about the same thing. I don't want to close your bug if it's still happening. Would you mind providing specific steps to reproduce the problem now so I can try to reproduce it too?

Comment 9

[adamtokarsky]

Sure.

Initial state: https://imgur.com/2MyY1Jg
Move mouse to the center of the poorly-drawn X. Left-click. Result: https://imgur.com/yWjfyeR
Down-arrow key five times. Result: https://imgur.com/vTmEgUp
Shift+delete. Result: https://imgur.com/0VvqdvQ
Up-arrow key four times. Result: https://imgur.com/936CA7O (Don't mind the extra result at the bottom of the list; it populated to fill the gap as I was pasting/cropping back and forth)
Shift+delete. Result (no change): https://imgur.com/Hrnvo8i

Comment 10

[Drew Willcoxon :adw]

Thanks, I think I know what's happening. You've removed the top sites from your new tab, right? So you are in fact seeing the old history view, which still has the problem, which I can still reproduce. The stack is the same as in comment 2 but the line numbers have changed:

Failed to find the selected result in the results UrlbarController.jsm:598
    _handleDeleteEntry resource:///modules/UrlbarController.jsm:598
    handleKeyNavigation resource:///modules/UrlbarController.jsm:386
    _on_keydown resource:///modules/UrlbarInput.jsm:2251
    maybeDeferEvent resource:///modules/UrlbarEventBufferer.jsm:157
    _on_keydown resource:///modules/UrlbarInput.jsm:2250
    handleEvent resource:///modules/UrlbarInput.jsm:372

Comment 11

[Drew Willcoxon :adw]

Oh man, this is due to a simple error. We're checking index == 0 when we should be checking index < 0: https://searchfox.org/mozilla-central/rev/0e09b9191c02097034e46b193930f91c45b7885d/browser/components/urlbar/UrlbarController.jsm#603

I'll fix this real quick.

Comment 12

[adamtokarsky]

Neat! Also, you are correct--the only box I have checked for that setting is Highlights.

Comment 13

[adamtokarsky]

Well, "Highlights," and "Visited Pages" under it.

Comment 14

[Drew Willcoxon :adw]

Hmm, recently we changed it so we show "top sites" even when you've removed them from new tab, in bug 1634279 and bug 1623666. I'm not actually sure whether there's any way to trigger the old history view anymore. This comment suggests not. If there isn't, then I don't see how this bug can happen anymore. But we should still fix this error.

So if you disabled top sites on new tab because you didn't want to see them in the urlbar, that's bad news I guess.

Comment 15

[Drew Willcoxon :adw]

Attachment: Bug 1572329 - Fix error that prevented the top history result from being deleted.

AFAICT this bug isn't reproducible anymore after bug 1623666. i.e., it's no
longer possible to show the history view. You can still see a history-like view
by typing ^ or a space, but in that case the first result is a search result,
not a history result, so it can't be deleted anyway. Nevertheless we should fix
this error.

Comment 16

[adamtokarsky]

I'm not sure precisely what settings you're referring to with this Top Sites stuff, but I've taken the liberty of screengrabbing a number of potentially-relevant Options settings ( https://imgur.com/a/fxnfmfM ) to make them available for analysis if you so desire. If you'd like to go deeper on this it seems we should divert to another thread; I'll follow your lead on how(/if) to proceed with this line of inquiry.

Comment 17

[Drew Willcoxon :adw]

Thanks, I was just describing the problem in technical terms. But in summary, we have two different lists of "top sites" and they're not exactly the same. One is the list that's on the new tab page. The other is the one that you've screenshotted in this bug. Firefox has shown the second list in the urlbar forever, for example when you click the arrow button that you mentioned in comment 0 (and that no longer exists). We've basically deprecated the second list in favor of the first so that there is a single list of top sites in all of Firefox.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/49daf897f76c
Fix error that prevented the top history result from being deleted. r=mak

Comment 19

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/49daf897f76c

Comment 20

[Cristian Fogel [:cfogel]]

Confirmed issue with 78.0 on Windows 10.
Fix verified with 79.0b3 on Windows 10, macOS 10.15.5, Ubuntu 18.