1234258 - Cannot remove first entry from awesome bar

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P3)

Description

[Sven Kuenzler]

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:43.0) Gecko/20100101 Firefox/43.0
Build ID: 20151216175450

Steps to reproduce:

New tab
Open the awesomebar dropdown
Press cursor down to select the first entry
Hit <Del> or <Shift>-Del


Actual results:

The url of the selected entry is shown in the awesome bar
upon closing and reopening the drop down the entry to be deleted is still in the list


Expected results:

The entry should be gone from the dropdown
(in fact all subsequent entries can still be deleted using this procedure)

Comment 1

[Loic]

Do you speak about the search suggestions implemented in the awesome bar?
If yes, you can disable this new feature in about:config > browser.urlbar.unifiedcomplete=false.
http://superuser.com/questions/1014611/is-it-possible-to-get-old-suggestion-system-back-in-firefox-43-0

Comment 2

[Marco Bonardo [:mak]]

(In reply to Loic from comment #1)
> Do you speak about the search suggestions implemented in the awesome bar?
> If yes, you can disable this new feature in about:config >
> browser.urlbar.unifiedcomplete=false.

This pref will go away asap, better to use a userChrome.css.

Comment 3

[Sven Kuenzler]

No, I mean the entries that show up before you type anything to the bar, just by activating the dropdown. The most recently used (or "top" recently used?) addresses are shown in that case I believe.

Comment 4

[Marco Bonardo [:mak]]

it's the most "frecent" typed entries.

This is interesting, I think it's a regression due to the fact in most cases the first entry is special, but not in this case.

Comment 5

[Loic]

Reg range:
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=152eafb9dc89a1d37ca5e24ee19d1987a66f1201&tochange=907891e1c2112a703439aa885d3e47ac905878ff

Drew Willcoxon — Bug 1176437 - Don't show fallback "Search with" result when the awesomebar is empty. r=mak

Comment 6

[Loic]

(In reply to Marco Bonardo [::mak] from comment #2)
> (In reply to Loic from comment #1)
> > Do you speak about the search suggestions implemented in the awesome bar?
> > If yes, you can disable this new feature in about:config >
> > browser.urlbar.unifiedcomplete=false.
> 
> This pref will go away asap, better to use a userChrome.css.

Marco, do you have a bug report about that, please?

Comment 7

[Marco Bonardo [:mak]]

(In reply to Loic from comment #6)
> Marco, do you have a bug report about that, please?

about what, removing the pref? bug 995095.
It's a natural process for all the new features, the pref is not an opt-out, it's there just to disable the feature if something breaks along the way to release.

Comment 9

[Marc Auslander]

It's not just the first entry - at least in 44.0b4.  Select any entry and delete - it is deleted but it remains and the following entry disappears from the list.

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Drew, do you think you will be able to fix that for 44? Thanks
Tracking for 45 because it is an user-facing regression

Comment 11

[Drew Willcoxon :adw]

Yes, I think I should have a patch soon.

Comment 12

[Drew Willcoxon :adw]

Attachment: patch

The problem is in urlbarBinding's handleDelete.  It calls mController.handleText() when popup.selectedIndex == 0 instead of calling handleDelete().  In the case of this bug, when the selected index is 0, it's a normal result that should be able to be deleted.

This patch changes that conditional so that it checks whether the selected result is the heuristic result.

Comment 13

[Drew Willcoxon :adw]

(In reply to Marc Auslander from comment #9)
> It's not just the first entry - at least in 44.0b4.  Select any entry and
> delete - it is deleted but it remains and the following entry disappears
> from the list.

I can't reproduce that on 44.0b4.  Sometimes there's a slight delay as the entries in the popup rearrange themselves, which maybe makes it a little hard to tell what's going on?

Comment 14

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=95988d378c2a

Comment 15

[Drew Willcoxon :adw]

Attachment: patch v1.1

Previous patch had test failures because it was trying to call _isFirstResultHeuristic on the input itself in handleDelete when it should have been calling it on this.popup.

Comment 16

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=48f2f0bc9805

Comment 17

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Seems like a basic scenario, tracked for FF44+

Comment 18

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/436e2013c50f

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/436e2013c50f

Comment 20

[Drew Willcoxon :adw]

Comment on attachment 8705354 [details] [diff] [review]
patch v1.1

Approval Request Comment
[Feature/regressing bug #]: UnifiedComplete + Bug 1176437
[User impact if declined]: Can't delete the first awesomebar entry
[Describe test coverage new/current, TreeHerder]: Existing tests (although they didn't catch this bug)
[Risks and why]: Low-medium risk, touches awesomebar code, but this patch is small and well understood
[String/UUID change made/needed]: None

Comment 21

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Sven, could you please verify this issue is fixed as expected on a latest Nightly build (1/12/2016)? Thanks!

Comment 22

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Florin, would you be able to verify this fix on a Nightly build? The patch has a bit more code change than I would be comfortable uplifting right away and need your team's verification before I can take this to Beta44. I believe this is a release blocking scenario as such. Please let me know!

Comment 23

[Marc Auslander]

I just tested https://hg.mozilla.org/mozilla-central/rev/e790bba372f14241addda469a4bdb7ab00786ab3

My bug 1236364 which was duped to this bug is not fixed in that build.

What I see is that after getting a list of potential matches from history by typing a match phrase in the address bar, if I hover over any entry, including the first, and press delete, the hovered over entry is still visible and the subsequent entry disappears.  But if I then carefully do NOT try to visit anything in the list but start again, I find that the hovered over entry is in fact gone from history after all!

Note that this appears, when hovering over the first entry, as the bug described here, but it isn't - the first entry is deleted but the list is incorrectly updated.  Of course if you then visit that entry, it is re-entered so you might think it was not deleted.

Comment 24

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

(In reply to Marc Auslander from comment #23)
> I just tested
> https://hg.mozilla.org/mozilla-central/rev/
> e790bba372f14241addda469a4bdb7ab00786ab3
> 
> My bug 1236364 which was duped to this bug is not fixed in that build.
> 
> What I see is that after getting a list of potential matches from history by
> typing a match phrase in the address bar, if I hover over any entry,
> including the first, and press delete, the hovered over entry is still
> visible and the subsequent entry disappears.  But if I then carefully do NOT
> try to visit anything in the list but start again, I find that the hovered
> over entry is in fact gone from history after all!
> 
> Note that this appears, when hovering over the first entry, as the bug
> described here, but it isn't - the first entry is deleted but the list is
> incorrectly updated.  Of course if you then visit that entry, it is
> re-entered so you might think it was not deleted.

Marc, sorry for the confusion but the fix will be in Nightly 1/13/2015 build. Please verify on the next Nightly build and let us know if this is still an issue. Thanks!

Comment 25

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Sigh! I meant 1/13/2016 Nightly build.

Comment 26

[Drew Willcoxon :adw]

(In reply to Marc Auslander from comment #23)
> I just tested
> https://hg.mozilla.org/mozilla-central/rev/
> e790bba372f14241addda469a4bdb7ab00786ab3
> 
> My bug 1236364 which was duped to this bug is not fixed in that build.

Thanks for explaining more about what you're seeing.  I see it too.  It's a different bug.  I'm going to undupe your other bug now that we know more about what's going on.

Comment 27

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Assigning to Andrei to find someone to verify this, and do some additional exploratory testing around it.

Comment 28

[Petruta Horea [:phorea], Desktop Test Engineering]

Verified using Nightly 46.0a1 2015-01-13 under Win 7 64-bit, Ubuntu 14.04 64-bit and Mac OS X 10.9.5.
In this build, the first entry is deleted, as expected.

Drew, please see below two potential issues and let me know if they should be filled (I found them during exploratory and are not regressions from this patch):

1. Open the awesomebar dropdown and delete all entries using the Del key -> The last entry url remains visible in the location bar. Opening a new tab and then returning deletes the entry from location bar.

2. Choose to see the search suggestions and enter a term.
2.1 Go to the third entry from suggestions list (the url is filled with that suggestion) and delete it -> the next entry is focused, but url bar is not updated (still showing the deleted suggestion)
2.2 Deleting all suggestions, will leave the first selected entry in the url bar, while the drop down only shows the first entry (entered term - Search with "default engine")

Comment 29

[Drew Willcoxon :adw]

Thanks Petruta, could you please file new bugs?

Comment 30

[Marc Auslander]

this sounds similar to bug 1236364

Comment 31

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8705354 [details] [diff] [review]
patch v1.1

I am denying this uplift to Beta44 because:
1. This issue (though a mainline scenario) existed in Fx43. So it is not a dot release candidate by itself.
2. Based on SV's testing and comment 23 and 28, I believe there are several (corner case) issues in deleting entries from awesome bar. This would be best dealt in one-go for Fx45 release.
3. The patch is touching a pretty core area and has a medium risk associated.

Let's take this in Aurora45 though.

Comment 32

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/dcfbfa1f35d9

Comment 33

[Sven Kuenzler]

Tested with 46 nightly setup (Windows). Works as expected for me in that build.

Thanks, Sven

Comment 34

[Sylvestre Ledru [:Sylvestre]]

Thanks for the confirmation!

Comment 35

[Petruta Horea [:phorea], Desktop Test Engineering]

(In reply to Drew Willcoxon :adw from comment #29)
> Thanks Petruta, could you please file new bugs?

Logged bug 1239662 and bug 1239663.

Thanks, Drew!