Closed Bug 412027 Opened 17 years ago Closed 17 years ago

Map Esc to the cancel button in the Bookmark contextual dialog

Categories

(Firefox :: Bookmarks & History, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 3 beta3

People

(Reporter: zeniko, Assigned: asaf)

References

Details

(Keywords: regression)

Attachments

(2 files)

From bug 393509 comment #23:
While bookmarking a page has become easier for mouse users (click on the star and you're done), it hasn't changed much for keyboard users (it's still [Ctrl+D, Enter]). What has changed though:

1. Hit Ctrl+D
2. Change your mind
3. Hit Esc

Expected result:
No bookmark was added.

Actual result:
At step 3 you could just as well have hit Enter. Not adding the bookmark has now become a tab-feast to reach the "Delete" button (see bug 409623).

Getting the new popup may be fine when the page has already been bookmarked, but when it hasn't, the new popup doesn't add hardly any additional value over the classic Add Bookmark prompt (the tagging bits being the exception, but they could easily be added) while regressing years of muscle memory.

Note: This wouldn't be as bad if [Ctrl+D] was changed to simply added a bookmark without any fuss (same as clicking the star the first time) and opened the new popup on subsequent [Ctrl+D]s.
Beltzner and I were just discussing this earlier. I think he said there were existing bugs on improving that behavior.
Or maybe he just said there are existing plans to improve it but no bugs, I forget.
Dup of bug 396513?
(In reply to comment #3)
Not as long as that one is WONTFIX and Beltzner talks about fixing this... ;-)

Besides: That bug focuses more about what Esc should mean in the popup while this one here is about the keyboard interaction in general (i.e. there are different ways to fix it, e.g. reusing the classic prompt or not displaying a popup at all on Ctrl+D).
(In reply to comment #4)
>or not displaying a popup at all on Ctrl+D.

Oh boy, that would be very bad as all browsers I've worked with have had this shortcut. I don't really want Firefox to change it. Things were so good!
>Beltzner and I were just discussing this earlier. I think he said there were
>existing bugs on improving that behavior.

The tracking bug for this interface is bug 393509.

The new UI that I will be posting soon will still be instant apply, but will support undo.  I think we should map enter to close, and ESC to undo, so that they keyboard interactions will effectively remain the same.  The only difference is that now the bookmark is created after control-d, as opposed to after enter.  Either way esc gets rid of it.
(In reply to comment #6)
> The new UI that I will be posting soon will still be instant apply, but will
> support undo.

Alex: To reiterate my point: Why does Ctrl+D have to open the new Star-popup at all?

Why can't it lead to the Add Bookmark prompt from Firefox 2.0? Adding a bookmark (with or without modifying its title and storage location) would still be the very same key sequence [Ctrl+D, Enter]!

OTOH you seem to insist that keyboard users have to hit at least two key combinations (first Ctrl+D, then at least Enter) but restrict my options without any obvious reason (other than that it was just easier to implement). If I have to hit Enter anyway, why not let [Esc] do something different (especially when it's always done so far)?

And just to make it clear: I'm all for [Esc] closing the Star-popup (as [Esc] just closes any other popup). The Star-popup is thus just not the right choice for keyboard users!

Dang, I feel so not understood... morphing this bug's title.

Two options:
1. Always display the "Add Bookmark" dialog (would allow to create several bookmarks of the same page).
2. Only display the "Add Bookmark" dialog when the page isn't bookmarked and have Ctrl+D otherwise display the Star-popup as well.
Summary: bookmark adding regression for keyboard users ([Ctrl+D, Esc] now equals [Ctrl+D, Enter]) → Use "Add Bookmark" on Ctrl+D instead of the Star-popup (otherwise [Ctrl+D, Esc] equals [Ctrl+D, Enter])
>Dang, I feel so not understood... morphing this bug's title.

Really sorry about that, I get the specific interface you would like, but can you clarify exactly why you want it?

A keyboard only interaction for bookmarking (control-d, enter) is faster than having to use the mouse, and faster is better. That makes sense and I agree.

Hitting ESC should undo the action and close out the window, because supporting undo puts the user in control. That also makes sense and I agree.

The only remaining functional difference between the traditional add bookmark dialog and the new contextual dialog (aside from visual styling, location on the screen, etc.) is that you can create multiple bookmarks of the same page with the add bookmark dialog, and you can't with the contextual dialog.

Two questions:

1) what is the use case for creating multiple bookmarks of the same page, and in what situations is copying and pasting in the library window not sufficient for this use case?

2) Are there any other difference between the two interfaces that I'm missing that still lead you to feel that the previous interface is better?

Again my apologies for you not feeling understood, I'm trying to make sure I understand all of your reasoning since I'm currently working on the new design.
(In reply to comment #8)
Thanks for hearing me out!

> 1) what is the use case for creating multiple bookmarks of the same page, and
> in what situations is copying and pasting in the library window not sufficient
> for this use case?

Besides tradition I don't see much benefit in being able to bookmark a page twice. That's why I'd be fine with Ctrl+D opening the Star-popup when the page is already(!) bookmarked. So repeatedly hitting Ctrl+D would be equivalent to clicking the Star twice.

Bonus points for also changing "Bookmarks -> Bookmark This Page…   Ctrl+D" to "Bookmarks -> Edit This Page's Bookmark   Ctrl+D" (or something alike) for already bookmarked pages, should we follow this route.

> 2) Are there any other difference between the two interfaces that I'm missing
> that still lead you to feel that the previous interface is better?

The missing accesskeys in the Star-popup (cf. bug 409623). The other way round it's the missing Tag editor (could be done in bug 411261). Those are both fixable.

The main difference is thus that when you accidentally hit Ctrl+D (instead of Ctrl+F or Ctrl+E) [Esc] will always cancel the action without any side effects.
(In reply to bug 412387 comment #10)
> In the new design it will really be more of a notification.

Except that it still can't be ignored... (which makes all the difference): As long as a keyboard user has to hit [Enter] or [Esc] after [Ctrl+D] before being able to continue working, the newly proposed change (iteration 8) is still suboptimal.

Better alternatives :

1. Instead of the star popup display a proper notification bar stating "This page has been bookmarked" and containing an [Edit] button. Hitting that button or hitting Ctrl+D a second time will then display the popup. That would make adding a bookmark a one click operation for both keyboard and mouse users.

2. Give the Star-popup a second state "You just added a bookmark, please confirm" in which [Esc] means the same as "Undo and close" instead of just "Close". This state would be used only when a user hits Ctrl+D and the page hasn't been bookmarked already.

3. As proposed in comment #9: The first time, Ctrl+D displays the classic dialog and on repeated keypresses we get the Star-popup.

Otherwise I as a keyboard user I feel somewhat a second-class citizen opposed to mouse users - which will be the vast majority of users, sure, but as I've repeatedly tried to outline above, there's should be no real incompatibility here.

Alex: Have you ever tried to unplug the mouse for a day...?
So, having had another look at iteration 8, [Esc] should indeed work as expected - although without knowing this, it's not obvious at all (at least to me [Esc] means "Cancel" and not "Undo").

OTOH now I'm not sure whether [Ctrl+D, Enter] still works as expected, as there's no default button which Enter could trigger...

Finally, keyboard users now need _another_ keypress to edit the bookmark.

To me this unfortunately still feels like regression wrt stream-lining. With Firefox 2.0 Ctrl+D led you directly to a place where you could (1) edit, (2) confirm or (3) cancel the bookmark to be created. Currently only (1) and (2) are possible, with iteration 8 it will be (2) and (3). *sigh*

(In reply to bug 393509 comment #45)
> >In essense, this *is* a normal dialog
> 
> In that both this and normal dialogs grab keyboard focus.  The key difference
> is that this dialog tells you "your done" while normal dialogs say "fill out
> this form."

And as to stream-lining: "your done" dialogs are of the most unproductive sort. That's what ignorable notification bars are for!
(In reply to comment #10)
> Alex: Have you ever tried to unplug the mouse for a day...?

Note that this can also be useful as a sanity check for accessibility. Making the UI workable without a mouse is more than a gimmick for users that prefer to use the keyboard.
>Note that this can also be useful as a sanity check for accessibility

Yeah, I was about to say the same thing.  Talking with beltzner this morning, we think it makes the most sense for control-d to open the expanded state of the contextual dialog (full mockup here: http://people.mozilla.com/~faaborg/files/granParadisoUI/places_NewBookmarkDialog_i8.png)  Esc will map to a cancel button that is being added in i9, so functionally this is going to be the same as the Firefox 2 dialog for keyboard users.  There are of course some aesthetic differences in the location of the dialog on the screen, and it's natural mapping to the star icon, but those differences won't be noticeable to anyone using a screen reader.
Summary: Use "Add Bookmark" on Ctrl+D instead of the Star-popup (otherwise [Ctrl+D, Esc] equals [Ctrl+D, Enter]) → Map Esc to the cancel button in the Bookmark contextual dialog
Don't eat my ESC
Assignee: nobody → mano
Status: NEW → ASSIGNED
Attachment #300405 - Flags: review?(gavin.sharp)
Attachment #300405 - Flags: approval1.9b3?
Attachment #300405 - Flags: approval1.9?
Attachment #300405 - Attachment is patch: true
Attachment #300405 - Attachment mime type: application/octet-stream → text/plain
Attachment #300405 - Flags: review?(gavin.sharp) → review+
Attachment #300405 - Flags: approval1.9b3?
Attachment #300405 - Flags: approval1.9b3+
Attachment #300405 - Flags: approval1.9?
Attachment #300405 - Flags: approval1.9+
mozilla/browser/base/content/browser.xul 1.419
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Would you mind not marking bugs as FIXED which aren't? This bug is about a functional regression for keyboard users which either gets a proper fix or a WONTFIX.

BTW: Your patch seems to make dismissing the "Bookmark Removed" popup more difficult to dismiss (I have to tab to the Undo button first before Esc works).
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
OS: Windows XP → All
Hardware: PC → All
Attached patch Some fixesSplinter Review
* Fix "Bookmark this link"
 * Fix the ESC-not-working issue.
 * When opening right into the edit state (Accel+D or Bookmark This Frame/Link), make Cancel/ESC also remove the bookmark.
 * In that state, hide the Remove Bookmark button.
Attachment #300444 - Flags: review?(dietrich)
Comment on attachment 300444 [details] [diff] [review]
Some fixes


>   cancelButtonOnCommand: function SU_cancelButtonOnCommand() {
>-    PlacesUtils.ptm.endBatch();
>-    this._batching = false;
>-    PlacesUtils.ptm.undoTransaction();
>+    this.endBatch();
>+    PlacesUtils.ptm.undoTransaction(); 

nit: whitespace

r=me
Attachment #300444 - Flags: review?(dietrich) → review+
Comment on attachment 300444 [details] [diff] [review]
Some fixes

a=beltzner for cleanup of functionality that landed for code freeze
Attachment #300444 - Flags: approval1.9b3+
mozilla/browser/base/content/browser-places.js 1.88
mozilla/browser/base/content/nsContextMenu.js 1.34
Status: REOPENED → RESOLVED
Closed: 17 years ago17 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 beta3
When using 'Bookmark this link' the pop-up Dialog box is not positioned correctly, its set as left-justified, and should be under the 'Star' for conformity. 

Ctrl+D or Bookmark this page opens the Dialog under the 'Star'.

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b3pre) Gecko/2008013013 Minefield/3.0b3pre Firefox/3.0 ID:2008013013
(In reply to comment #22)
> When using 'Bookmark this link' the pop-up Dialog box is not positioned
> correctly

That's bug 405339.
Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.2pre) Gecko/2008072104 GranParadiso/3.0.2pre ID:2008072104
Status: RESOLVED → VERIFIED
Depends on: 471302
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: