show bookmark URL over mouseover of bookmark toolbar item in status bar

RESOLVED FIXED in Firefox 3 beta4

Status

()

Firefox
Bookmarks & History
P4
enhancement
RESOLVED FIXED
14 years ago
9 years ago

People

(Reporter: Chris Neale, Assigned: Michael Schonfeld)

Tracking

unspecified
Firefox 3 beta4
Points:
---
Bug Flags:
blocking0.9 -
blocking-aviary1.0 -
blocking-aviary1.5 -
blocking-firefox3 -
wanted-firefox3 +
blocking-firefox2 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 8 obsolete attachments)

(Reporter)

Description

14 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5a cdn) Gecko/20030718 Mozilla Firebird/0.6
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5a cdn) Gecko/20030718 Mozilla Firebird/0.6

Mozilla Firebird implementation of bug 23460 : Bookmarks URL should show in
status bar at mouseover of menuitem

Reproducible: Always

Steps to Reproduce:
1. Move mouse over a bookmark on the Personal toolbar
2.
3.

Actual Results:  
see Status text does not change to URI of bookmark

Expected Results:  
URI of bookmark shown in statusbar
(Reporter)

Comment 1

14 years ago
Created attachment 128069 [details] [diff] [review]
Patch for browser.xul / bookmarksMenu.js to show URI of bookmarks in statusbar

based on Tim Powell's patch for Seamonkey :
http://worldtimzone.com/mozilla/bugs/bug23460/
Confirming, patch looks good.
Status: UNCONFIRMED → NEW
Ever confirmed: true
taking QA contact, sorry about the bugspam
QA Contact: asa → mconnor

Comment 4

14 years ago
I saw this fixed in Bonsai about weeks ago but finally got a build. Mr QA please
verify since this should've been marked a while ago and I don't understand why
it's not marked.
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
thanks, I saw this fixed, but in my little "clean up after hyatt" rounds I
missed some :)
Status: RESOLVED → VERIFIED
(Reporter)

Comment 6

14 years ago
did Bonsai report that 

mozilla/browser/base/content/browser.xul or
mozilla/browser/components/bookmarks/content/bookmarksMenu.js had been altered ?

afaict this particular bug hasn't been touched

those checkins must have been for Seamonkey
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 7

14 years ago
Comment on attachment 128069 [details] [diff] [review]
Patch for browser.xul / bookmarksMenu.js to show URI of bookmarks in statusbar

browser.xul change in bookmarksMenu.js change not in afaict
Attachment #128069 - Attachment is obsolete: true
(Reporter)

Comment 8

14 years ago
Created attachment 130654 [details] [diff] [review]
patch for bookmarksMenu.js
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/browser/base/content&command=DIFF_FRAMESET&file=browser-menubar.inc&rev1=1.1&rev2=1.2&root=/cvsroot
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/browser/base/content&command=DIFF_FRAMESET&file=browser.xul&rev1=1.201&rev2=1.202&root=/cvsroot
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/toolkit/content/widgets&command=DIFF_FRAMESET&file=toolbar.xml&rev1=1.26&rev2=1.27&root=/cvsroot

Chris, hyatt checked in at 08/12/2003 18:55, however he did this in browser.xml
and toolbar.xml, which is why the files in question didn't change.

Re-resolving

This is actually working in current builds.
Status: REOPENED → RESOLVED
Last Resolved: 14 years ago14 years ago
Resolution: --- → FIXED
Okay, I see, you want all mouseovers of bookmarks in any menu to do this.  Hyatt
intended only for bookmarks in the Bookmarks menu to have this, which is what he
implemented.  The BTF bookmarks do not trigger the statusbar change.

Reopening for now.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 11

14 years ago
hyatt has a time machine ?

as for on other menuitems, I'm not sure

I missed the other implementation, only tried after you pointed it out

haven't followed bonsai for a bit, just been blindly building dailies

Comment 12

14 years ago
is comment 10 the only thing left for this bug?
Summary: Mozilla Firebird : Bookmarks URL should show in status bar at mouseover of menuitem → show bookmark URL over mouseover of bookmark toolbar item in status bar

Comment 13

14 years ago
This should also apply to bookmarks inside folders on the bookmarks toolbar.
*** Bug 237451 has been marked as a duplicate of this bug. ***

Updated

13 years ago
Flags: blocking0.9?
not a 0.9 blocker, plussing for 1.0 to get on review radar
Flags: blocking1.0+
Flags: blocking0.9?
Flags: blocking0.9-

Comment 16

13 years ago
Bug 239429 is this bug's cousin in the Bookmarks sidebar
Priority: -- → P4

Comment 17

13 years ago
Created attachment 151923 [details] [diff] [review]
Patch for bookmarks toolbar menus and buttons

Following Hyatt's example, I created a fix that uses XUL/XBL. This extends
statusbar support to toolbarbuttons. This could be used to enhance the back and
forward button and their submenus so that they'd show the URL in the statusbar
(bug 88541 for Mozilla--is there one for Firefox?) much like the Go menu.

This was patched against the files in the chrome jars for Firefox 0.9. I
believe toolbarbutton.xml has a different location in the source tree.

Updated

13 years ago
Attachment #130654 - Attachment is obsolete: true
(Reporter)

Comment 18

13 years ago
Created attachment 152062 [details] [diff] [review]
reworking of attachment 151923 [details] [diff] [review] so that it alters files in AVIARY_1_0_20040515_BRANCH branch of cvs tree
(Reporter)

Comment 19

13 years ago
Created attachment 152063 [details] [diff] [review]
diffed too soon, should have cvs upped first
Attachment #152062 - Attachment is obsolete: true

Comment 20

13 years ago
Thanks, Chris. The patch in comment #19 looks to be the right files and the same
changes as my original patch. What's next? Who can review this?

Comment 21

13 years ago
-> vlad. There's a patch in here.
Assignee: p_ch → vladimir
Status: REOPENED → NEW

Comment 22

13 years ago
p4 priority - not a blocker. if a fully reviewed patch materializes, please
nominate for aviary approval. 
Flags: blocking-aviary1.0+ → blocking-aviary1.0-
*** Bug 265297 has been marked as a duplicate of this bug. ***

Comment 24

13 years ago
Patch needs update because of recent Livemarks changes. I've applied it manually
to FF PR, and it works as expected.
*** Bug 268409 has been marked as a duplicate of this bug. ***
Assignee: vladimir → vladimir+bm
(Reporter)

Comment 26

13 years ago
Created attachment 167774 [details] [diff] [review]
Update of attachment 152063 [details] [diff] [review] - takes Livemarks into account

May be woolly
Attachment #152063 - Attachment is obsolete: true

Comment 27

13 years ago
(In reply to comment #26)
> Created an attachment (id=167774) [edit]
> Update of attachment 152063 [details] [diff] [review] [edit] - takes Livemarks into account
> 
> May be woolly

Great job again! The patch applies to FF 1.0 with no problems. Can you set
review flag on the patch, so the patch can be reviewed and checked in?
(Reporter)

Updated

13 years ago
Attachment #167774 - Flags: review?(vladimir+bm)

Updated

13 years ago
Flags: blocking-aviary1.1+
(Reporter)

Updated

13 years ago
Flags: blocking-aviary1.1+ → blocking-aviary1.1?

Comment 28

12 years ago
As of today (Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b2) Gecko/20050523
Firefox/1.0+), the urls for bookmarks (top-level as well as bookmarks contained
in a folder, or sub-folder, etc.) are still not shown in the 'status bar'. At
least on arch: PC, OS: Linux.

Neither does the feature as mentioned in comment #17.
(In reply to comment #28)
> As of today (Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b2) Gecko/20050523
> Firefox/1.0+), the urls for bookmarks (top-level as well as bookmarks contained
> in a folder, or sub-folder, etc.) are still not shown in the 'status bar'. At
> least on arch: PC, OS: Linux.

hm, odd: using 2005052311-trunk (deer park alpha 1) on linux fc3 (gnome), I see
the urls for bookmarks in the bookmarks main menu (both toplevel and children of
bookmark folders).

however, I don't see the urls of bookmarks in either the bookmarks toolbar or
the bookmarks sidebar, when I do a mouseover.

Comment 30

12 years ago
(In reply to comment #29)

> hm, odd: using 2005052311-trunk (deer park alpha 1) on linux fc3 (gnome), I see
> the urls for bookmarks in the bookmarks main menu (both toplevel and children of
> bookmark folders).
> 
> however, I don't see the urls of bookmarks in either the bookmarks toolbar or
> the bookmarks sidebar, when I do a mouseover.

Yes, that's exactly what I am talking about. URLs are not desplayed in the
'status bar' when hovering over bookmarks in the 'bookmarks toolbar'.

This is with a trunk checkout from about 5 hours ago.
Sairuh, this was established a long time ago.  Hyatt implemented for the menu
only, and never got the other bits done. :-/

Comment 32

12 years ago
Ok, I should have a patch ready by the end of the week (say, monday evening,
just for the case...).

Comment 33

12 years ago
(In reply to comment #32)

> Ok, I should have a patch ready by the end of the week (say, monday evening,
> just for the case...).

So, I already start talking to myself...

Sorry for the delay, but <rant>why is there no possibility to add multiple
bindings independent of each other?? It's simply a pain in the ass to always
track back every possible binding chain, and then insert the new binding into
this hierarchy. But even worse, this prevents code reuse, and renders binding
hierarchies basicly unreadable. It is simply not possible to add any orthogonal
functionality, which does not have to be linked into the existing binding
hierarchy. Of course I see the issues of multiple inheritance, but this could be
solved by linking the additional bindings transparently into the hierarchy. And
don't tell me this is what document.addBinding is for. This little bitch simply
crashes when called (ah, who said bug
https://bugzilla.mozilla.org/show_bug.cgi?id=51261 was fixed??)!</rant>

Now, I first have to workaround this bug, maybe by adding events manually. If
someone could point out a better way to add a binding, please speak up.

Updated

12 years ago
Depends on: 296375
if 296375 materialized in time to get a low-risk patch before beta, we can take
a look, but this isn't a blocker.
Flags: blocking-aviary1.1? → blocking-aviary1.1-
BTW: Take a look at your review request again, it says "(BM, do not Cc or
Request) ", so you won't get any review from vlad this way (try requesting
review from :vlad).
*** Bug 312796 has been marked as a duplicate of this bug. ***
Assignee: vladimir+bm → nobody

Updated

11 years ago
Flags: blocking-firefox2?
Not a blocker, this is a small polish fix at best.
Flags: blocking-firefox2? → blocking-firefox2-
Comment on attachment 167774 [details] [diff] [review]
Update of attachment 152063 [details] [diff] [review] - takes Livemarks into account

this patch is no longer applicable (needs to be written against Places)
Attachment #167774 - Attachment is obsolete: true
Attachment #167774 - Flags: review?(vladimir+bm)

Comment 39

11 years ago
Created attachment 227674 [details] [diff] [review]
Updated patch against MOZILLA_1_8_BRANCH

I've updated the previous patch against MOZILLA_1_8_BRANCH. There is still time for it to make into FF2.0.
Attachment #227674 - Flags: review?(mconnor)

Comment 40

11 years ago
Created attachment 234523 [details] [diff] [review]
Updated patch against latest MOZILLA_1_8_BRANCH only.

This is an updated patch for FF2. It has a minimal regression risk and addresses an annoying usability issue. Please consider before FF final ships out.
Attachment #227674 - Attachment is obsolete: true
Attachment #234523 - Flags: review?(gavin.sharp)
Attachment #227674 - Flags: review?(mconnor)
QA Contact: mconnor → bookmarks
Comment on attachment 234523 [details] [diff] [review]
Updated patch against latest MOZILLA_1_8_BRANCH only.

I can't review this - you likely want to ask "mconnor@moz" or "bugs.mano@".
Attachment #234523 - Flags: review?(gavin.sharp)

Updated

11 years ago
Attachment #234523 - Flags: review?(bugs.mano)
Comment on attachment 234523 [details] [diff] [review]
Updated patch against latest MOZILLA_1_8_BRANCH only.

We shouldn't change the  toolbarbutton binding in order to implement something like this (there are statusbar utils in browser.js, setOverLink in particular). Also, considering the way this is implemented, the statustext might be overwritten while you've hovering an item.

Using setOverLink should address these issues.
Attachment #234523 - Flags: review?(bugs.mano) → review-

Updated

10 years ago
Flags: blocking-firefox3?

Updated

10 years ago
Flags: blocking-firefox3? → blocking-firefox3-
Whiteboard: [wanted-firefox3]

Updated

10 years ago
Flags: wanted-firefox3+
Whiteboard: [wanted-firefox3]
(Assignee)

Comment 43

10 years ago
Created attachment 294510 [details] [diff] [review]
patch

Please note: The patch will not show the URI of a menuitem due to a different bug. Will open a followup.
Attachment #294510 - Flags: review?(dietrich)
(Assignee)

Updated

10 years ago
Attachment #294510 - Attachment filename: 213613.diff → 213163.diff
Comment on attachment 294510 [details] [diff] [review]
patch

>Index: browser/components/places/content/toolbar.xml
>===================================================================
>RCS file: /cvsroot/mozilla/browser/components/places/content/toolbar.xml,v
>retrieving revision 1.112
>diff -u -p -8 -r1.112 toolbar.xml
>--- browser/components/places/content/toolbar.xml	14 Dec 2007 21:10:38 -0000	1.112
>+++ browser/components/places/content/toolbar.xml	25 Dec 2007 01:37:03 -0000
>@@ -992,16 +992,27 @@
>             parent = parent.parentNode;
>           }
>           return false;
>         ]]></body>
>       </method>
>     </implementation>
> 
>     <handlers>
>+      <handler event="mouseover"><![CDATA[
>+        var button = event.target;
>+        if (button.parentNode == this && button.node &&
>+            PlacesUtils.nodeIsURI(button.node))
>+          window.XULBrowserWindow.setOverLink(event.target.node.uri, null);
>+        else
>+          window.XULBrowserWindow.setOverLink("", null);

I think the mouseout handler covers the else case.

r=mano with that removed..
Attachment #294510 - Flags: review?(dietrich) → review+
(Assignee)

Comment 45

9 years ago
Created attachment 301586 [details] [diff] [review]
v1.2

Thanks for the review, Mano!

Same deal, no {else}. 
r=mano.
Attachment #294510 - Attachment is obsolete: true
Attachment #301586 - Flags: approval1.9?

Updated

9 years ago
Attachment #301586 - Flags: approval1.9? → approval1.9+
(Assignee)

Comment 46

9 years ago
Created attachment 301727 [details] [diff] [review]
For checkin
Attachment #301586 - Attachment is obsolete: true
mozilla/browser/components/places/content/toolbar.xml 1.120
Assignee: nobody → dev
Target Milestone: --- → Firefox 3 beta4
Status: NEW → RESOLVED
Last Resolved: 14 years ago9 years ago
Resolution: --- → FIXED

Comment 48

9 years ago
I think that "Home" button URL should be shown too, but now I see "Done" in status bar when mouse over Home bookmark.
It's fixed for items directly on the bookmarks toolbar, but items within sub-folders (inc. Smart Bookmarks and feeds) still do not give status bar feedback.
I'm assuming this is what the note in comment #43 is referring to (as it almost certainly is), but, as it's not completely clear, I'll say it anyway.
(Assignee)

Comment 50

9 years ago
William:  Yea, that's gonna be Bug 409748.

Comment 51

9 years ago
So the URL now displays in the bookmarks menu and bookmarks toolbar.
Are you going to implement this for the bookmarks sidebar as well (bug 239429)?
(Assignee)

Comment 52

9 years ago
Actually, it only shows the URL for the bookmarks toolbar, not menu.  Maybe we should open a follow-up bug for the bookmarks menu.
(Assignee)

Comment 53

9 years ago
Opened a follow-up bug regarding both the bookmarks menu, and the toolbars chevron.  Bug 417938.
You need to log in before you can comment on or make changes to this bug.