379003 - Make tab tooltips more detailed

Status: NEW

(Firefox :: Tabbed Browser, enhancement, P4)

Description

[Michael Ventnor]

I have a feeling there is a duplicate bug for this, but I can't find it. The closest is bug 345378, but that covers the all tabs menu which I believe is a bad choice.

I have managed to overhaul the tab tooltip so that the user can get the page title, URL and a generated preview at a glance. Hover over any tab and you get a preview of it.

Will commit screenshot soon (once the triagers are sure this bug isn't a dupe)

Comment 1

[Ria Klaassen (not reading all bugmail)]

Looks like Bug 308492.

Comment 2

[Michael Ventnor]

(In reply to comment #1)
> Looks like Bug 308492.
> 

No, that's for tab expose.

Comment 3

[Michael Ventnor]

Attachment: Screenshot

Here is a couple of screenshots of the patch. The top one is a typical case, the bottom one shows off some UI decisions I made that I'd like some comments on.

The page title is bolded and wrapped, and will never be cropped, so the user can get the entire title at a glance. Since the URL is less user-friendly than the title, the URL underneath it is cropped, but it is set to end-cropping since the most interesting bits of the URL are at the start (eg domain). Also, URL's rarely contain whitespace and thus can't be wrapped AFAIK.

Instead of getting the preview at the top of the page like Opera, it gets the preview starting from the scroll position at that tab. This, among other things, make sure that the preview is as close to the tab content's current state as possible. It's not live like Expose though, that would be too slow.

Screenshot says it all, though.

Comment 4

[Michael Ventnor]

Attachment: Patch

Forgot to mention, screenshots on Windows don't include the cursor. :) Niel, you can review this right? It only has to be code-wise for now, but I'm sure Beltzner won't ask for too much.

Comment 5

[Steffen Wilberg]

> Since the URL is less user-friendly than
> the title, the URL underneath it is cropped, but it is set to end-cropping
> since the most interesting bits of the URL are at the start (eg domain).
Imagine www.microsoft.com.some-pretty-long-text.evil.com.
I'd suggest to always show at least the effective top-level domain, like .co.uk, plus one level below ("eTLD+1"), like bbc.co.uk, and maybe even highlight that.
See http://mxr.mozilla.org/seamonkey/source/netwerk/dns/public/nsIEffectiveTLDService.idl

Comment 6

[Michael Ventnor]

(In reply to comment #5)
> Imagine www.microsoft.com.some-pretty-long-text.evil.com.
> I'd suggest to always show at least the effective top-level domain, like
> .co.uk, plus one level below ("eTLD+1"), like bbc.co.uk, and maybe even
> highlight that.
> See
> http://mxr.mozilla.org/seamonkey/source/netwerk/dns/public/nsIEffectiveTLDService.idl
> 

1) Is there any way to forcibly wrap a label? Most URLs don't contain whitespace.
2) The URL on the popup is an extra detail, the main thing the popup shows is the title and preview. The user can still get the full URL from the location bar. Plus, the only way the user can be tricked by the popup is if they visit the site in the first place, stay on there in a background tab, and pay no attention to the location bar whatsoever.
3) Isn't that what Phishing protection is for? ;)
4) The popup isn't always shown like the location bar. I really doubt it provides a new phishing vector.

Comment 7

[Ria Klaassen (not reading all bugmail)]

I like the way Seamonkey it does, with the address on top. Although the tooltips take considerably longer to appear than with the Firefox Tab Preview extension where the tooltip beats the click.

Comment 8

[Steffen Wilberg]

Sure, there are other places in the UI which are much more important to highlight the "eTLD+1", especially the location bar, as discussed in the newsgroups.
But making sure that the "eTLD+1" is visible is much better than simply end-cropping. Maybe we should implement a generic way to do that, like crop="url", outside of this bug of course.

Comment 9

[Michael Ventnor]

Attachment: Patch with manual cropping

Incorporate Steffen's suggestion; the domain must always be visible. Otherwise nothing else has changed behaviour-wise.

Comment 10

[Neil Deakin]

Comment on attachment 263090 [details] [diff] [review]
Patch with manual cropping


I only skimmed the patch. I didn't review it in detail. Since this is a fairly noticeable UI feature, it should have a UI review done first, or be approved for Firefox 3 

+          <![CDATA[
+            const CROP_LENGTH = 37;
+            if (aURL.length <= CROP_LENGTH)
+              return aURL;
+
+            var uriObj = Components.classes["@mozilla.org/network/io-service;1"]
+                           .getService(Components.interfaces.nsIIOService)
+                           .newURI(aURL, null, null);
+            var urlSubstr = aURL.substr(0, Math.max(uriObj.prePath.length + 2, CROP_LENGTH));

Why are you creating an nsIURI? Is there some processing/normalization/etc that needs to occur?

Also, why does the crop attribute not suffice for cropping the url?

+              closeLabel.removeAttribute("hidden");
+              toolContent.setAttribute("hidden", "true");

Use .hidden instead. Similar elsewhere.

+            if (titleField.hasChildNodes())
+              titleField.removeChild(titleField.firstChild);
+            titleField.appendChild(document.createTextNode(tabWindow.document.title ||
+                                                           tn.getAttribute("label")));

Just set titleField.textContent to the text

Comment 11

[Michael Ventnor]

(In reply to comment #10)
> 
> +          <![CDATA[
> +            const CROP_LENGTH = 37;
> +            if (aURL.length <= CROP_LENGTH)
> +              return aURL;
> +
> +            var uriObj =
> Components.classes["@mozilla.org/network/io-service;1"]
> +                           .getService(Components.interfaces.nsIIOService)
> +                           .newURI(aURL, null, null);
> +            var urlSubstr = aURL.substr(0, Math.max(uriObj.prePath.length + 2,
> CROP_LENGTH));
> 
> Why are you creating an nsIURI? Is there some processing/normalization/etc that
> needs to occur?
> 
> Also, why does the crop attribute not suffice for cropping the url?

I did, but then Steffen voiced his concerns about URL spoofing (comment 5, though I really doubt something like this provides a new spoofing hole). That code just makes sure the domain is always visible no matter what (the previous patch did use the crop attribute).

Comment 12

[Dão Gottwald [:dao]]

Comment on attachment 263013 [details]
Screenshot

You're wasting a lot of space here. Shouldn't the image and the text be arranged vertically?

Comment 13

[Dão Gottwald [:dao]]

You should consider porting the SeaMonkey code. There's no point in reinventing the wheel.

Besides, I find preview images as part of tooltips disturbing. Switching to the tab is generally faster, let alone that you can hardly gather useful information from the preview images. I think this should be left to extensions.

Comment 14

[Michael Ventnor]

That sounds reasonable...

Forgetting about the previews for now, another goal I wanted was to get rid of the cropping of the tab name, and making the tooltip more detailed overall. Maybe I should just change the tooltip to be like in the screenshot minus the preview.

Comment 15

[Michael Ventnor]

Attachment: Patch without the preview

I'm convinced that previews are a bit unnecessary, however I still want to make the tooltip more useful and detailed. This patch is like the screenshot but without the preview.

I'll ask for a review on this once Beltzner gives his ui-r.

Comment 16

[Michael Ventnor]

Attachment: New screenshot

Comment 17

[Michael Ventnor]

Attachment: Updated patch

Comment 18

[Michael Ventnor]

Comment on attachment 265088 [details] [diff] [review]
Updated patch

I'll just request review now while waiting for Beltzner's ui-r.

Comment 19

[Asaf Romano (gone)]

Comment on attachment 265088 [details] [diff] [review]
Updated patch

Please set the request once beltzner reviews the mockup.

Comment 21

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

Comment on attachment 265087 [details]
New screenshot

Yeah, let's try this. I'm a little curious to see how well it handles large URLs in the wild, so we should keep an eye on it when it lands and think about how to polish the presentation to be most useful.

Comment 22

[Dão Gottwald [:dao]]

Michael, time to request review? This needs an updated patch, though.

Comment 23

[Michael Ventnor]

I think this is kind of made redundant now with the addition of multi-line tooltips from a while ago. Unless we want to have canvas previews again I don't think this is needed anymore.

That, and last time I tried this there was a problem where if the URL changed while the tooltip was shown for that tab, the tooltip would stay around until you clicked on it. I couldn't figure out how to fix that, but maybe it fixed itself eventually. I don't know.

Comment 24

[Dão Gottwald [:dao]]

Back to the default assignee then. I'll leave this bug open, since there is a positive ui-review.

Comment 26

[Mike Connor [:mconnor]]

I think this is interesting, though I think tab preview is a better user experience.

Comment 28

[Mike de Boer [:mikedeboer]]

Attachment: Rich tooltip example

Comment 29

[Mike de Boer [:mikedeboer]]

Madahava posted this screenshot in bug 924888, in which he touches on this feature in light of Australis.