155710 - Need Undo in the URL bar

Status: VERIFIED FIXED

(Camino Graveyard :: Toolbars & Menus, defect)

Description

[Simon Fraser [no longer active]]

We have to get Undo working in the URL bar somehow.

Comment 1

[David Haas (gone, not reading mail)]

Do you want to be able to Redo actions, too, or just undo?

Getting undo to work is a new 1 line method.  Redo's going to take
more heartache and grief.

Comment 2

[Simon Fraser [no longer active]]

Just Undo is better than nothing. Long-term, I'd like to see Undo and Redo.

Comment 3

[David Haas (gone, not reading mail)]

Sorry - should have asked this the first time.  

do you want undo to go letter-by-letter that you typed in, or
right back to the initial text value?

(letter by letter is tougher to do right - have huge problems
getting the first letter)

Comment 4

[David Haas (gone, not reading mail)]

Attachment: Undo only

Patch for implementing undo in URL bar.

There are problems here.  1st: since it picks up text changes right after
they've occured, rather than right before they occur, first time you hit
undo nothing happens.  2nd: since the undo selector is for a different
method than it's getting called in, redo doesn't work. 3rd:  had to add crud 
so we didn't generate redo's which don't do anything.  If 1 was fixed, I
think 2 would fix itself, which would mean we wouldn't need 3.	But my little
brain can't think of how to do that today.

Comment 5

[Mike Pinkerton (not reading bugmail)]

you want it, you got it ;) -> sfraser

Comment 6

[Simon Fraser [no longer active]]

Oh man, why is this so hard? Why doesn't cocoa provide automatic undo? As for
what should be undone, it should behave like undo in a normal text editor. A
series of typed characters should merge into one undo operation. A delete should
close that undo batch, and start a new one, etc.

Comment 7

[David Haas (gone, not reading mail)]

Actually, Cocoa does provide automatic undo/redo - in NSTextView, not NSTextField.

I thought about it this weekend & came to the conclusion that switching the URL
bar to TextView from TextField would be the best way to do this.  But I was
hoping you'd come up with a better idea.  

I haven't really looked at what all would need to be changed to make this
happen.    I think I'll look right now.

Comment 8

[David Haas (gone, not reading mail)]

Attachment: undo & redo

Wow - it turned out to be not very complicated at all.	This patch replaces the
normal window field editor sent to the URL bar (an NSText) with one of its
subclasses (an NSTextView) which happens to have built in Undo & Redo.

There's 2 or 3 different places one can substitute field editors - in the
window, the window controller, and the text field cell - the window controller
was the easiest place.	Feel free to disagree.

It also looks like if you don't save the field editor, auto complete doesn't
work.  So I made it a member variable of the window controller.

Another thing:	I think this patch will also end up fixing 149198.  

Final thing: odd strings can result if you start typing, choose an auto
complete string, then undo, since the auto complete stuff sets a value in the
text field (which it shouldn't do - it should just set values in the field
editor).  I'll make a fix later, if nobody else does first.

Comment 9

[David Haas (gone, not reading mail)]

Attachment: alternate undo & redo

While the other patch worked, I think this is cleaner.

I replaced the NSTextField in the URL bar with an NSTextView.
This moves all the handling for undo & redo, as well as handling
the escape key, into the AutoCompleteTextView class, instead of
scattered in the BrowserWindowController & BrowserWindow classes.
Plus, this gets rid of a couple extra/unused member variables in
classes, eliminates the need for a separate BrowserWindow class,
and funky Field Editor handling.  

If you use this, I'd change the name of CHAutoCompleteTextField.h or .mm
to CHAutoCompleteTextView.h & mm.  Just a suggestion.

Comment 10

[David Haas (gone, not reading mail)]

Attachment: BrowserWindow.nib

Sorry it's a nib not a diff.

This goes with the "Alternate undo & redo" patch.

Comment 11

[David Haas (gone, not reading mail)]

Comment on attachment 90901 [details] [diff] [review]
alternate undo & redo

Ack.  Found some bugs in this.

Comment 12

[David Haas (gone, not reading mail)]

Attachment: this is it

It gets no better than this one.

If you have problems merging in the BrowserWindow.nib stuff from the patch,
I'll upload a zipped copy of it.

Comment 13

[Garret Huntress]

Just a note, whenever this bug gets closed, should probably also close bug
149198 as appearently the fix for that bug lies here.

Comment 14

[Mike Pinkerton (not reading bugmail)]

what changes need to be made to the bw.nib? i can just do them manually if it's
easy.

taking back as this fixes a crasher of mine

Comment 15

[Mike Pinkerton (not reading bugmail)]

hrm, i tried hooking up the nib myself, but was thwarted. it doesn't work too
well. I just made a custom view with the correct custom class and hooked it all
up, but the autocomplete widget won't display properly and if i cmd-l into the
text view it won't let me type.

ideas?

Comment 16

[David Haas (gone, not reading mail)]

well, here's what i did.

i deleted the NSTextField.  I added a custom view, and made it a
CHAutoCompleteTextView.  I clicked on the custom view, then went to Layout ->
Make Subviews of -> Scroll View.  At that point, it worked, but the sizing was
all wrong.

I have the CHLocationBar autosizing on the top & bottom of outer box &
horizontal of inner box.

I have the scroll view autosizing on the top, bottom, right of the outer box &
horizontal of the inner box

I have the CHAutoCompleteTextView not autosizing on the outer box & horizontal &
vertical sizing on the inner box.

Hope this helps

Comment 17

[David Haas (gone, not reading mail)]

Attachment: fixed cmd-L behavior

Add this to the other patch to make cmd-L work.

Comment 18

[Mike Pinkerton (not reading bugmail)]

better, but there's still 2 things i don't like about the new behavior:

1) you see the url bar selection when it doesn't have focus
2) opening a new window with about:blank should select all the text in the url
bar, it doesn't. the insertion point is put after.

Comment 19

[David Haas (gone, not reading mail)]

Attachment: fixes about:blank & funky selection

you have to apply this after "this is it" and "fixes cmd-L behavior"

Comment 20

[Mike Pinkerton (not reading bugmail)]

+  if ([[self getCurrentURLSpec] isEqualToString: @"about:blank"])
+    [mWindowController focusURLBar];

this bothers me a little...we already have code in the mainController (line 165)
that should be doing this. Why do we need it in a 2nd location?

Comment 21

[David Haas (gone, not reading mail)]

Why do we need it in this 2nd location?  Why, because it makes it work ;)

Whenever the browser wrapper's OnLoadingComplete (or whatever it is) gets
called, it unselects the text view and puts the insertion point at the end.  If
we don't refocus here, you don't get about:blank highlighted.  I played around
with this for a couple of hours trying to figure out what was happening, but I
still don't know.  I wondered if it had something to do with toolbar validation,
but commenting out all the validation code didn't seem to fix it.  

If you've got any better ideas, let me know.  But I was stumped, so I declared
victory and ran.

Comment 22

[Mike Pinkerton (not reading bugmail)]

i reworked our url loading code so that we only load about:blank once (instead
of twice). the 2nd load was messing up our focus setting. with that fixed, this
patch is good to go (w/out your last setting of the urlbar focus, of course).
landing soon...

Comment 23

[Mike Pinkerton (not reading bugmail)]

landed, though there's 1 problem. if you autocomplete and then undo, the url
gets jumbled. i'll file a new bug for that. i think it's minor.

Comment 24

[Winnie Lam]

verified with 07-29 build on OS 10.1.5.

Undo menu item works, as well as the keyboard shortcut. 

I do see the autocomplete bug in pink's comment 23.

marking verified.