87969 - pasting in URL to document window should add URL to urlbar history

Status: RESOLVED FIXED

(Core Graveyard :: History: Global, defect)

Description

[Jeremy M. Dolan]

A very handy Nav feature on UNIX, middleclicking to paste a URL into the
document part of the window, but this should add the URL to the location bar
history, since the middle click is really just a short cut for going to the
location bar, clearing it, and then pasting.

In netscape 4 I *OFTEN* found myself trying to find a site in the location bar
history, only to remember that I had used the middle-click method to access it,
and I had to dig through 1000s of pages in the generic History to find it again.

Comment 1

[Alec Flett]

a good idea... eventually we'll be merging global and urlbar history, and
flagging certain global history entries as "entered on the url bar"

Comment 2

[Alec Flett]

reassigning history bugs to new owner - send this bug back to me if it looks
like something I should fix (such as embedding-related architecture issues),
rather than the actual history owner...

Comment 3

[Jeremy M. Dolan]

3-month ping.

Did they get merged yet? Still keep losing stuff I pasted in... annoying.

Comment 4

[Blake Ross]

Attachment: patch (untested)

Comment 5

[Jeremy M. Dolan]

Patch doesn't seem to do anything. No JS error, no history entry.

Comment 6

[Wolfgang Rosenauer [:wolfiR]]

Attachment: FF patch

Comment 7

[Wolfgang Rosenauer [:wolfiR]]

Attachment: improved FF patch

Comment 8

[Mike Connor [:mconnor]]

Comment on attachment 187919 [details] [diff] [review]
improved FF patch

>+# vim:set ts=2 sw=2 sts=2 et:

we don't add vim modelines, its evil or something

>-function addToUrlbarHistory()
>+function addToUrlbarHistory( aUrlToAdd )

drop the spaces around the arg, please.

>+  try {
>+    addToUrlbarHistory(url);
>+  } catch (ex) {
>+    // Things may go wrong when adding url to session history,
>+    // but don't let that interfere with the loading of the url.
>+  }
>+

the try/catch bits in addToUrlbarHistory() should already catch the likely
failures, but the other caller wraps the call too.  Seems like overkill, but
I'm not going to worry about it at this point.

Comment 9

[Wolfgang Rosenauer [:wolfiR]]

Attachment: patch with integrated review comments

Comment 10

[Wolfgang Rosenauer [:wolfiR]]

Attachment: seamonkey patch

Comment 11

[Wolfgang Rosenauer [:wolfiR]]

Attachment: seamonkey patch #2

urlbar history handling is rather different from FF.

Comment 12

[[e-mail]]

Comment on attachment 187923 [details] [diff] [review]
seamonkey patch #2

Unfortunately you can't make contentAreaClick.js depend on sessionHistoryUI.js
because it's used by components other than navigator.

Comment 13

[Wolfgang Rosenauer [:wolfiR]]

Attachment: seamonkey #3

then I think this is the only possible way?

Comment 14

[[e-mail]]

Comment on attachment 188099 [details] [diff] [review]
seamonkey #3

OK, but you should remove the sessionHistoryUI.js version and fix up its
callers. Also you should add the url before the call to getShortcutOrURI
because that's the way the other callers do it.

Comment 15

[Wolfgang Rosenauer [:wolfiR]]

Attachment: seamonkey #4

this patch moves the addToUrlbarHistory() function to
communicator/contentAreaClick.js and use the URL as argument.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I checked in attachment 187920 [details] [diff] [review].

Comment 17

[Wolfgang Rosenauer [:wolfiR]]

Comment on attachment 188218 [details] [diff] [review]
seamonkey #4

not really sure if sr is needed but Neil is an sr for the case.
This patch is seamonkey only stuff.

Comment 18

[Wolfgang Rosenauer [:wolfiR]]

Checking in browser/resources/content/navigator.js;
/cvsroot/mozilla/xpfe/browser/resources/content/navigator.js,v  <--  navigator.js
new revision: 1.573; previous revision: 1.572
done
Checking in browser/resources/content/navigator.xul;
/cvsroot/mozilla/xpfe/browser/resources/content/navigator.xul,v  <--  navigator.xul
new revision: 1.431; previous revision: 1.430
done
Checking in browser/resources/content/sessionHistoryUI.js;
/cvsroot/mozilla/xpfe/browser/resources/content/sessionHistoryUI.js,v  <-- 
sessionHistoryUI.js
new revision: 1.54; previous revision: 1.53
done
Checking in communicator/resources/content/contentAreaClick.js;
/cvsroot/mozilla/xpfe/communicator/resources/content/contentAreaClick.js,v  <--
 contentAreaClick.js
new revision: 1.48; previous revision: 1.47
done