Closed Bug 1234258 Opened 4 years ago Closed 4 years ago

Cannot remove first entry from awesome bar

Categories

(Firefox :: Address Bar, defect, P3)

43 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 46
Tracking Status
firefox43 --- wontfix
firefox44 + wontfix
firefox45 + fixed
firefox46 + verified

People

(Reporter: svenk, Assigned: adw)

References

Details

(Keywords: regression, Whiteboard: [fxsearch][unifiedcomplete])

Attachments

(1 file, 1 obsolete file)

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)
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
Component: Untriaged → Location Bar
Flags: needinfo?(svenk)
(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.
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.
Flags: needinfo?(svenk)
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
Priority: -- → P3
Whiteboard: [fxsearch][unifiedcomplete]
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
(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?
(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.
Assignee: nobody → adw
Status: NEW → ASSIGNED
Flags: needinfo?(adw)
Duplicate of this bug: 1236364
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.
Drew, do you think you will be able to fix that for 44? Thanks
Tracking for 45 because it is an user-facing regression
Flags: needinfo?(adw)
Yes, I think I should have a patch soon.
Flags: needinfo?(adw)
Attached patch patch (obsolete) — Splinter Review
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.
Attachment #8704880 - Flags: review?(mak77)
(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?
Attached patch patch v1.1Splinter Review
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.
Attachment #8704880 - Attachment is obsolete: true
Attachment #8704880 - Flags: review?(mak77)
Attachment #8705354 - Flags: review?(mak77)
Seems like a basic scenario, tracked for FF44+
Attachment #8705354 - Flags: review?(mak77) → review+
https://hg.mozilla.org/mozilla-central/rev/436e2013c50f
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
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
Attachment #8705354 - Flags: approval-mozilla-beta?
Attachment #8705354 - Flags: approval-mozilla-aurora?
Sven, could you please verify this issue is fixed as expected on a latest Nightly build (1/12/2016)? Thanks!
Flags: needinfo?(svenk)
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!
Flags: needinfo?(florin.mezei)
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.
(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!
Flags: needinfo?(marcausl)
Sigh! I meant 1/13/2016 Nightly build.
(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.
Assigning to Andrei to find someone to verify this, and do some additional exploratory testing around it.
Flags: needinfo?(florin.mezei) → needinfo?(andrei.vaida)
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")
Flags: needinfo?(andrei.vaida)
Thanks Petruta, could you please file new bugs?
this sounds similar to bug 1236364
Flags: needinfo?(marcausl)
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.
Attachment #8705354 - Flags: approval-mozilla-beta?
Attachment #8705354 - Flags: approval-mozilla-beta-
Attachment #8705354 - Flags: approval-mozilla-aurora?
Attachment #8705354 - Flags: approval-mozilla-aurora+
Tested with 46 nightly setup (Windows). Works as expected for me in that build.

Thanks, Sven
Flags: needinfo?(svenk)
Thanks for the confirmation!
Status: RESOLVED → VERIFIED
(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!
You need to log in before you can comment on or make changes to this bug.