88541 - Show URI in status bar onmouseover of Back/Forward menu items

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect, P3)

Description

[Ashley Bischoff (blog at handcoding.com)]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.1+)
Gecko/20010627
BuildID:    2001062704

When right-clicking on the back button, a list appears of previously visited
pages. However, when mousing over that list, the url for the respective page
should appear in the status bar.

Reproducible: Always
Steps to Reproduce:
1. Load a page, for example: http://www.cnn.com
2. Click one of the article links on the front page
3. Once the article loads, right click on the back button
4. Move the mouse down to the entry for CNN.com Homepage

Actual Results:  The status bar doesn't change when mousing over the links in
the back-button history.

Expected Results:  The status bar should show the url for the link, in this
case: http://www.cnn.com

I searched for dups, but I didn't come up with any. And, I wasn't entirely sure
of the correct component.

Comment 1

[Ashley Bischoff (blog at handcoding.com)]

Added keyword "4xp", since this bug doesn't occur on NN 4.x

Comment 2

[Francisco León]

Should this be an enhancement request? Confirmed

Comment 3

[Alec Flett]

if it _doesn't_ work in 4.x then we don't put the keyword there! the keyword
means that it _did_ work in 4.x!

Comment 4

[timeless]

this is 4xp because the statusbar text changes in nc4 but not in mozilla. but 
that's ok because it's also a dupe which was blocked or futured because we 
didn't support something at some point. 

Comment 5

[:deanis74]

Can't have this until we fix bug 63847.  Or maybe this should depend on bug 
47434.  Anyway, you get the idea.

(removing DUPEME from status whiteboard based on this)

Comment 6

[mdimitrio]

I see no mention to left clicking the down arrow, which is the same as right
clicking anywhere on the back button. Just don't forget to see if both are
working when implementing this feature. Summary should change to reflect this as
well.

Marcos.

Comment 7

[Ashley Bischoff (blog at handcoding.com)]

Marcos:

Not wanting to end up with a hugely-long Summary, what wording do you suggest?

Comment 8

[mdimitrio]

The most descriptive summary would be:

"Right-click back-button (or left-click on arrow) history should show urls in
status bar"

But the best and shorter would be:
"Back-button history should show urls in status bar"

Problem with this one is the way the bug is described (being specific on the
right click, forgetting about left click on arrow). People can still make some
confusion.

So my suggestion is this, change the report to include "left click on arrow" as
well and use the shorter summary. I don't know if this is possible in bugzilla,
if not, we could fill another, better described, bug.

Marcos.

Comment 9

[Ashley Bischoff (blog at handcoding.com)]

Old summary:
"Right-click back-button history should show urls in status bar"

New summary:
"Activating back-button history should show urls in status bar (either through
right-click or left-click-on-arrow)"

Marcos: A compromise, eh?

Comment 10

[mdimitrio]

Good. Perhaps we should change it a bit though, because as it's now it might
give the impression that you want "show urls in status bar *by* either through
right-click or left-click-on-arrow". That really isn't the intended behavior.

So my suggestion is as follow:

"Back-button history (activated either through right-click or
left-click-on-arrow) should show urls in status bar"

Hey, and the best part? It's one character shorter than the current summary. :-)

Marcos.

Comment 11

[Ashley Bischoff (blog at handcoding.com)]

Old summary:
"Activating back-button history should show urls in status bar (either through
right-click or left-click-on-arrow)"

New summary:
"Back-button history (activated either through right-click or
left-click-on-arrow) should show urls in status bar"

Marcos: I think that's enough for the re-summarizing ;). I wouldn't want to
annoy any of the other people CC'd on this bug.

Comment 12

[timeless]

This applies to Go> too...

Comment 13

[mdimitrio]

Yes, and don't forget Bookmarks too, activated either by clicking on the menu
item, or on the Bookmarks button.

Marcos.

Comment 14

[Alec Flett]

argh.. take two at reassigning history bugs to new owner

Comment 15

[Blake Ross]

I do not believe the majority of users care what url they're going back or
forward to.

Comment 16

[Ashley Bischoff (blog at handcoding.com)]

Mpt: What's your take on the WONTFIX status here?

As I see it, this feature was included in NS 4.x, and I would think that the
developers had a rationale behind including it at the time :-/.

Comment 17

[mdimitrio]

Blake Ross wrote on 2001-10-12 15:24

> I do not believe the majority of users care what url they're going back or
forward to.

I disagree. It's the same as hovering over any link on web pages, you expect to
see the URL, to see where does that take you. It would be as useful to see the
URL in the status bar when hovering over items on the back button, bookmark list
(menubar and toolbar) and the Go menu.

Marcos.

Comment 18

[Francisco León]

Please reopen. There are lots of pages with the same title, and the only way to
tell between them is to look at the url below

Comment 19

[timeless]

considering our history story has some really whacky rules for what goes into 
history and that we have strange behavior wrt loading pages from said history i 
think it's reasonable to want to be able to see the url for the page.

assignee: do you have a specific number of average users who you'd like to hear 
from who want this behavior before you'd consider it wanted by normal people?

Perhaps we should change the back button feature to not have a drop down, if 
users don't really care what's in the pages they're going back to, then they 
can slowly skip through them until they find what they didn't really care to 
look for.

REOPENING, some non developing bugzilla users want it, some developing bugzilla 
users want it (i'm among them), and the nc4 team chose to include it.

Comment 20

[mdimitrio]

[e-mail] 2001-10-14 18:48 -------

>REOPENING...

Yes, thanks.

>Perhaps we should change the back button feature to not have a drop down, if 
users don't really care what's in the pages they're going back to, then they 
can slowly skip through them until they find what they didn't really care to 
look for.

I disagree, for many reasons. It's a time saver. Suppose I'm doing a research.
Do a search at google, and browse away on the same window. If I go 10 levels
deep, and want to come back to the google search page, then what? I would have
to click 10 times the back button. Then the question would come up, "Where's the
drop down menu on the back button? Netscape always had it...". Second, it's a
standard in the web browser interfaces. Every browser that I know of has it.
Third, we're trying to predict that all users behave in that way, pressing the
back button over and over, and that may be wrong.

Marcos.

Comment 21

[Jesse Ruderman]

See also bug 23460, a similar bug for the bookmarks menu.

Comment 22

[Matthew T (active 1999-2002)]

Amusingly, MSIE for Mac *only* shows the URI, not the page title, in its Back 
and Forward menus. I guess its designers overreacted to the problem of multiple 
pages on the same site having the same TITLE. I don't think Mozilla should do 
that, but it definitely should show the URI in the status bar, as it does for 
links.

Comment 23

[mdimitrio]

This should be fixed before 1.0 IMHO, because it seems to be easy to fix.

Marcos.

Comment 24

[Dan Soper]

cc-ing 'cos I'd kinda like to see this implemented. :-)

Frequently a site will give all its pages the same title, so the back-menu
becomes a guessing game if you're trying to get back to a specific page.

Comment 25

[Paulo Eduardo Neves]

*** Bug 244588 has been marked as a duplicate of this bug. ***

Comment 27

[Ed Lee :Mardak]

Copying polish flags like bug 63107.

Comment 28

[Ed Lee :Mardak]

Attachment: screenshot of v1

Screenshot showing hovering over history item producing the uri in the status bar.

Comment 29

[Ed Lee :Mardak]

Attachment: v1

Use DOMMenuItemActive/Inactive to set/clear the status bar with setOverLink.

Comment 30

[Simon Bünzli]

Comment on attachment 353646 [details] [diff] [review]
v1

>+function FillHistoryMenu(aParent, aHiding) {

We'll probably rather want two functions (Fill- and HideHistoryMenu).

>+  let addRemove = (aHiding ? "remove" : "add") + "EventListener";

A case of over-optimization (not too readable)? Then again:

>+  aParent[addRemove]("DOMMenuItemActive", function(aEvent)
>+    XULBrowserWindow.setOverLink(sessionHistory.getEntryAtIndex(
>+      aEvent.target.getAttribute("index"), false).URI.spec), false);

AFAICT you'll have to pass removeEventListener the exact same function instance that you passed addEventListener - and that's not what you get calling FillHistoryMenu twice (each function call creates a new one).

It might be sufficient, though to just add a listener the first time FillHistoryMenu is called and then leave it around so that it's automatically reused the next time.

Final issue: This only affects the popup you get when right-clicking the Back/Forward buttons, not the one available through the dropmarker (which is a child of the #unified-back-forward-button's dropmarker).

Comment 31

[Ed Lee :Mardak]

Attachment: v1.1

(In reply to comment #30)
> It might be sufficient, though to just add a listener the first time
> FillHistoryMenu is called and then leave it around so that it's automatically
> reused the next time.
Switched it to this and removed the removeEventListener and isHiding part, so no need for a separate function.

> Final issue: This only affects the popup you get when right-clicking the
> Back/Forward buttons, not the one available through the dropmarker (which is a
> child of the #unified-back-forward-button's dropmarker).
At least on OS X, this works for clicking the drop marker, right click, ctrl-click, and click-hold. However the dropdown is created, it's using FillHistoryMenu to populate the list, so it'll hit this code path.

Comment 32

[Alex Faaborg [:faaborg] (Firefox UX)]

Comment on attachment 353645 [details]
screenshot of v1

Looks good.  We should also be consistent that any time you are hovering and a downclick will navigate the browser, we show the URL that will be navigated to in the status bar.  So other examples would be the back and forward buttons themselves, the home button, and items in the bookmarks menu.  (Currently items in the bookmarks toolbar already display a URL in the status bar.)

We could pick up menu items like "release notes" for full internal consistency, but this is less important.

Comment 33

[Dão Gottwald [:dao]]

Comment on attachment 353982 [details] [diff] [review]
v1.1

>+  if (!aParent.hasAttribute("hasStatusListener")) {

Any reason why you're using an attribute rather than a property? Attribute names are usually all lowercase.

>+    aParent.addEventListener("DOMMenuItemInactive", function()
>
>+      XULBrowserWindow.setOverLink(""), false);

function() expr; is the short form of function() { return expr; }. I don't think it's suitable to use this if you don't want anything to be returned from the event listener. That's only asking for trouble when e.g. setOverLink is changed to return something.

Comment 34

[Ed Lee :Mardak]

Attachment: v1.2

(In reply to comment #33)
> >+  if (!aParent.hasAttribute("hasStatusListener")) {
> Any reason why you're using an attribute rather than a property?
Switched to plain javascript property. No particular reason for an attribute other than there's a hasAttribute method, but the hidden property being null would work just as well.

> >+    aParent.addEventListener("DOMMenuItemInactive", function()
> >+      XULBrowserWindow.setOverLink(""), false);
> function() expr; is the short form of function() { return expr; }.
Yeah I know. Switched to the full {} form to avoid issues if setOverLink does return something down the line.

Also, changed it to grab getWebNavigation().sessionHistory each time instead of the local var sessionHistory, so it grabs the right history for the current tab.

Comment 35

[Dão Gottwald [:dao]]

Comment on attachment 354966 [details] [diff] [review]
v1.2

>+      XULBrowserWindow.setOverLink(getWebNavigation().sessionHistory.
>+        getEntryAtIndex(aEvent.target.getAttribute("index"), false).URI.spec);

Can you just store the address on each item where they are created? We already have the session history entry there...

I think it might make sense to not show the address for the current page.

Comment 36

[Ed Lee :Mardak]

Attachment: v1.3

Set the uri on each entry. I also made it not show the uri for the current page by seeing if it's the checked item. (This is instead of grabbing getWebNavigation().sessionHistory.index every hovered item.)

Comment 37

[Dão Gottwald [:dao]]

Comment on attachment 354972 [details] [diff] [review]
v1.3

Although "Only the current page should have the checked attribute" is perfectly valid, you could avoid that detour by setting the uri attribute in the "if (j != index)" branch and testing for that attribute instead of "checked". And instead of "uri" it could be called "statusbarvalue" or so, to make the code more self-explanatory.

Comment 38

[Ed Lee :Mardak]

(In reply to comment #37)
> detour by setting the uri attribute in the "if (j != index)" branch
I was thinking about that, but I figured it would be more consistent if I set the uri on each entry.. and...

> And instead of "uri" it could be called "statusbarvalue"
Potentially other users might want to grab the value from the menu from an add-on, so giving it a generic name could be useful.


But then again, are there any plans down the line to have a special status bar value that's determined at list building time? Potentially we could say "Open <uri>" by default or "Open <uri> in new tab" if the user holds alt, etc. But that would be at hover-time not list-build-time, so "uri" instead of "statusbarvalue" would also fit.

Comment 39

[Ed Lee :Mardak]

Comment on attachment 354972 [details] [diff] [review]
v1.3

I'll need another r? for this patch, yeah?

Comment 40

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 354972 [details] [diff] [review]
v1.3

(transferring review to mconnor)

Comment 41

[Mike Connor [:mconnor]]

Comment on attachment 354972 [details] [diff] [review]
v1.3

fine by me.

Comment 42

[Ed Lee :Mardak]

http://hg.mozilla.org/mozilla-central/rev/5267ab4402c9

Comment 44

[Tracy Walker [:tracy]]

verified with: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090505 Minefield/3.6a1pre

Comment 45

[(no longer active)]

Is there any particular reason why we can't get an automated test for this patch?  It shouldn't be that hard (although I only glanced over the patch briefly.)

Comment 46

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 354972 [details] [diff] [review]
v1.3

I don't think we need to fix this in this version, and we're a little close to RC to take it.

Comment 47

[Alex Faaborg [:faaborg] (Firefox UX)]

This bug's priority relative to the set of other polish bugs is:
P2 - Polish issue that is in a secondary interface, occasionally encountered, and is easily identifiable.

Impacts the main window, but not incredibly common for users to look down and check where the back and forward buttons are going to take them.

Comment 48

[Nochum Sossonko [:Natch]]

I actually found this pretty annoying, the back/forward buttons are really the only way to get at history for this specific tab. Nom'ing for 1.9.1.x, although I'm not holding my breath...