340029 - elements styled with link-text should set status bar onmouseover

Status: NEW

(Toolkit :: UI Widgets, enhancement, P5)

Description

[Tony Chang (Google)]

It would be nice if labels with the link-text class set would show the link target in the status bar like HTML links.

Specifically, I want to use this on the links found in the anti-phishing warning bubble.

Comment 1

[Tony Chang (Google)]

Attachment: v1: set/restore status on mouse over/out

This is loosely based on the code in toolbar.xml that sets status text for menuitems.

Comment 2

[Simon Bünzli]

Nits:
* this.statusText is never set, so just use this.href.
* Why not use a statusbar property instead of hard-coding a value (since text-links are used outside of browser.xul as well)? You'll then have to make this feature opt-in, but that should be OK for chrome.

Comment 3

[Tony Chang (Google)]

(In reply to comment #2)
> Nits:
> * this.statusText is never set, so just use this.href.

statusText is to allow the user to have mouseover text if there's no href (e.g., an onclick event) or if you want to show something other than the href.

> * Why not use a statusbar property instead of hard-coding a value (since
> text-links are used outside of browser.xul as well)? You'll then have to make
> this feature opt-in, but that should be OK for chrome.
 
The statusbar is a property, it just has a default value of statusbar-display; users can still override it.  Maybe it shouldn't have a default value?

Comment 4

[Simon Bünzli]

(In reply to comment #3)
> statusText is to allow the user to have mouseover text if there's no href
> (e.g., an onclick event) or if you want to show something other than the href.

OK. In this case, please also add a "statustext" property (which does getAttribute/setAttribute) so that the statustext can also be set in the markup and not only from script.

> The statusbar is a property, it just has a default value of statusbar-display;
> users can still override it.  Maybe it shouldn't have a default value?

IMO it shouldn't. Since you can't know that the element of ID statusbar-display indeed is the statusbar (and not something else), I'd rather go with an opt-in than an opt-out property.

Comment 5

[Tony Chang (Google)]

Attachment: v2: set/restore status on mouse over/out

Make statusbar a property with no default value (opt-in required).

No need to add a statusText property, it's already on all XULElements: http://lxr.mozilla.org/seamonkey/source/dom/public/idl/xul/nsIDOMXULElement.idl#99

Comment 6

[Tony Chang (Google)]

Attachment: v3: set/restore status on mouse over/out

Oops, remove dump()

Comment 7

[Simon Bünzli]

Comment on attachment 225244 [details] [diff] [review]
v3: set/restore status on mouse over/out

This looks good to me (nit: drop the braces in restoreStatusText). However, I'm no reviewer.

Comment 8

[Simon Bünzli]

Comment on attachment 225244 [details] [diff] [review]
v3: set/restore status on mouse over/out

Tony: Seems that mconnor isn't really available for non-critical reviewing anymore. If you're still ready to work on this, please ask either Asaf Romano or Gavin Sharp for review. Sorry for the delay...

Comment 9

[BugBot [:suhaib / :marco]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:mstriemer, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 10

[Mark Striemer [:mstriemer]]

I think at this point we'd be better off removing MozTextLink and converting to HTML anchor elements.

Looks like we have 80 instances of is="text-link" [1]. I don't know if there's anything special going on here other than we would normally use XUL in XUL documents, but that's not always the case now.

[1] https://searchfox.org/mozilla-central/search?q=is%3D%22text-link%22

Comment 11

[Mark Striemer [:mstriemer]]

Looks like bug 1509393 is on file to do that work.