Closed Bug 171605 Opened 22 years ago Closed 20 years ago

Implement deletion of individual autocomplete results

Categories

(Firefox :: General, enhancement)

Product:
Firefox
Component:
General
Platform:
x86
All
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox0.9

People

(Reporter: bugzilla, Assigned: deanis74)

References

Details

Attachments

(2 files, 1 obsolete file)

no where near working
7.46 KB, patch
Details | Diff | Splinter Review
Hook this up for global history as well. Simplified the patch a bit, even.
22.52 KB, patch
bugs
: review+
Details | Diff | Splinter Review
Target Milestone: --- → Phoenix0.3
IE has Shift-Del shortcut for doing this. I'm not sure if it's a well-known fact
or not, but it's worth documenting it here anyway.
Actually just delete works in IE.
yes, selecting an item in the popup and hitting delete should remove it. I think
that's the cleanest method. 
What happens when you select an autocomplete item in a dropdown list? The item
is being displayed and focused in the edit area of a control and selected in the
autocomplete list at the same time. So, by just using Delete to remove
autocomplete item, one would both erase edit area, and delete autocomplete item.
Usually that is a two-step process. 

After autocomplete item deletion, edit portion of control should retain value.
The value should be selected to facilitate easy deletion if one desires. That's
why I mentioned modifier (Shift) in comment #1. Of course, that's my $.02.
See also bug 157749, bug 157751, and bug 157752.
Target Milestone: Phoenix0.3 → Phoenix0.4
Target Milestone: Phoenix0.4 → Phoenix0.5
Target Milestone: Phoenix0.5 → Phoenix0.6
Severity: normal → enhancement
OS: Windows XP → All
not gonna happen for 0.6
Target Milestone: Phoenix0.6 → Phoenix0.7
re: comment 4, IE doesn't fill the text into the field until you press Enter or
Tab, so it's not a problem there.  For our implementation, we still know what
the original text was so I'd think just revert to that.
Dean, do you want to take this?
Target Milestone: Firebird0.7 → Firebird0.8
Actually, I do.  I've already got some notes on how to do this.
Assignee: hewitt → dean_tessman
This is some work I did a little while back.  It crashes in RemoveEntryForRow,
probably because I'm passing in a completely invalid row.  I'm posting it here
because my back is messed up right now and I don't know when I'll be able to
sit at my computer again for an extended period of time.  Feel free to play
with this.
QA Contact: asa
This bug has quite a few votes, and it's targeted for 0.8. Is this blocking the
release, or ought it to be delayed to a later version?
Flags: blocking0.8?
not going to block the release for an enhancement request that doesn't have a
working patch or an active developer owning the bug
Flags: blocking0.8? → blocking0.8-
QA Contact: mpconnor
Kicking to 0.9 to get off the 0.8 Project Management Bugdar. Dean, if you do
decide to do this sooner, feel free to retarget.
Target Milestone: Firebird0.8 → Firebird0.9
*** Bug 230696 has been marked as a duplicate of this bug. ***
*** Bug 198327 has been marked as a duplicate of this bug. ***
*** Bug 234821 has been marked as a duplicate of this bug. ***
Comment on attachment 144175 [details] [diff] [review]
This should do it. Based on Dean's initial work.

Cool jst!  This looks much better than what I came up with.

>+    // Nothing left in the popup, close it.
>+    ClosePopup();

Shouldn't you call ClearSearchTimer() before ClosePopup()?  I can't remember
why I did it, but I know it had a reason.  It null-checks mTimer so it's safe
to call repeatedly.

>   case nsIDOMKeyEvent::DOM_VK_DELETE:
>+    {
>+      PRBool isShift = PR_FALSE;
>+      keyEvent->GetShiftKey(&isShift);
>+
>+      if (isShift) {
>+        mController->HandleDelete(&cancel);

So you have to press Shift+Delete to delete an entry?  Mine worked with only
Delete, which is how IE works as well.
Comment on attachment 144181 [details] [diff] [review]
Hook this up for global history as well. Simplified the patch a bit, even.

r=ben@mozilla.org

fantastic!
Attachment #144181 - Flags: review+
Yeah, this is Shift+Delete only, we talked this over with Ben and agreed to do
it this way since the delete key deletes the currently selected text, and we
didn't want to break that.

I'll add a line to clear that timer, not that I know that it's absolutely
required, but most other code does it, so I guess we should here too.

Thanks for having a look, and thanks Ben for the review!
Fix checked in. Marking FIXED.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
(In reply to comment #21)
> Yeah, this is Shift+Delete only, we talked this over with Ben and agreed to do
> it this way since the delete key deletes the currently selected text, and we
> didn't want to break that.

Why's that?  IE does it fine with only Delete.  It only kicks in if the popup is
open and there's an item selected in the list box.  That makes perfect sense to me.
It's because Delete is an acceptable way of forward-deleting the selection. 

try this:

Start typing:

www.fo|

this autocompletes:

  www.foo.com/
  www.foo.com/bar/
  www.foo.com/bar/foo/

you press down arrow twice, leaving yourself with:

      +- caret
      |
      v##selected##
www.fo|o.com/bar/
  www.foo.com/
##www.foo.com/bar/###############
  www.foo.com/bar/foo/

You hit delete to clear the selected text. Instead, www.foo.com/bar/ disappears.
Is that what you want to happen? 
The only argument I have is that it seems to work fine for IE, both in that I
haven't heard any complaints about the functionality and it works well for me. 
If you're navigating through the list, then something in the list is
highlighted.  You can then think of the list as having focus instead of the input.
I would have to say that what Internet Explorer uses in this case is a very moot
point; after all, this is a feature many people do not know even exists in
Internet Explorer at all.

While in most cases keeping things easy for "competitive upgrades" is a good
idea, in this case I would put forth that having the greatest functionality is
the most important.  That and actually mentioning in the help or something that
you can delete things from auto complete one-by-one. (believe it or not, some
people have been known to at least read parts of manuals at times.)

Regardless; I'm happy this bug has been fixed, and whether it uses shift or not
won't change that.

Thanks,
-[Unknown]
Personally I perfer what I did in the patch, but if someone has strong enoug
arguments and reasons to do what IE does (which I'm not used to, but it seems
bad), I can deal either way. But really, I do think what we now have is better
than what IE has. Less chance of accidental deletion from the db, which could be
bad.

Feel free to open new bugs or keep arguing here, but so far *I* prefer what we
now have.
> having the greatest functionality is the most important

True enough...

> after all, this is a feature many people do not know even exists in
> Internet Explorer at all.

If you like this bug, you will like bug 198327 even more... It will make the
whole thing more discoverable.
Users switching from IE will expect DEL. Shift + Del kind of complicates thing.

What is the exact reason that the text in the field is still selected when the
dropdown with the suggestions appears? I understand the argument that the DEL
key would delete that, but why not just have the focus in the dropdown? 
I would have to agree that DEL is the most logical given that many users will be
moving from MS IE to Mozilla/Firefox. The Shift-Del is not intuitive.

It would also be nice to have an option somewhere to clear out the list completely.
I've used it, I like it, I'm sold on Shift+Del.  Nice work jst - thanks for
running with this!
Status: RESOLVED → VERIFIED
> It would also be nice to have an option somewhere to clear out the list
> completely.


Taken as "Delete All" in bug 198327 comment 7 & bug 198327 comment 8.
I downloaded the daily build and tried it and the Shift-Del works very nicely. 
My only issue is that coming from IE one is used to Del working by itself,
however I can live with Shift-Del providing it is documented somewhere.

Thanks for fixing this and for all your hard work on this terrific tool.
I understand the reason not to use only the DEL button (comment 24), but why
does it have to select the text in the field that gets autocompleted?

Is there  any reason/benefit for this?
Doesn't violate CUA?  Shift-Delete means "Cut"

Doesn't this violate CUA?  Shift-Delete means "Cut"
*** Bug 239186 has been marked as a duplicate of this bug. ***
anyone know what CUA is?
CUA = Common User Access and quoting from IBM:

The Common User Access (CUA) establishes a degree of standardization that is 
compatible with the differences in the three environments and that supports 
transfer of users experiences. CUA is based on a user-interface architecture 
that identifies fundamental elements of structure. The intent is to provide a 
transfer of users conceptual-level learning across different and evolving 
technologies. CUA specifies user-interface components and guidelines to be used 
by application designers, and it provides a basis for programming development 
tool specifications.
CUA = Common User Access and was designed by IBM in the late 1980's.

Here is a quote from an IBM pub.

The Common User Access (CUA) establishes a degree of standardization that is 
compatible with the differences in the three environments and that supports 
transfer of users experiences. CUA is based on a user-interface architecture 
that identifies fundamental elements of structure. The intent is to provide a 
transfer of users conceptual-level learning across different and evolving 
technologies. CUA specifies user-interface components and guidelines to be used 
by application designers, and it provides a basis for programming development 
tool specifications.
I don't see Shift+Delete as being the current standard shortcut key for Cut in
Windows.  Most apps support it for backwards-compatibility only out of kindness.
Apps support Shift-Del as "Cut" because it is the standard.  The Ctl+C/X/V stuff
is a non-standard Mac thing, Windows only copied that to make Mac migrators happy.

I would still be interested to know the answer for my question in comment 34,
due to that seems be the the reason for shift + del and not just del.
I would suggest that people stop commenting in this bug about how this deletion
is done, open up a new bug if you want the implementation to change. This bug
was about implementing this feature, that's done, any further changes would IMO
be better tracked in a new bug.
*** Bug 239895 has been marked as a duplicate of this bug. ***
(In reply to comment #44)
> any further changes would IMO be better tracked in a new bug.

Bug 241774 was filed.


*** Bug 259516 has been marked as a duplicate of this bug. ***
Blocks: 330578
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: