Make tab tooltips more detailed

NEW
Unassigned

Status

()

P4
enhancement
12 years ago
a year ago

People

(Reporter: ventnor.bugzilla, Unassigned)

Tracking

Trunk
Future
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 5 obsolete attachments)

(Reporter)

Description

12 years ago
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)
Looks like Bug 308492.
(Reporter)

Comment 2

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

No, that's for tab expose.
(Reporter)

Comment 3

12 years ago
Created attachment 263013 [details]
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.
Assignee: nobody → ventnor.bugzilla
Status: NEW → ASSIGNED
Attachment #263013 - Flags: ui-review?(beltzner)
(Reporter)

Comment 4

12 years ago
Created attachment 263014 [details] [diff] [review]
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.
Attachment #263014 - Flags: review?(enndeakin)

Comment 5

12 years ago
> 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
(Reporter)

Comment 6

12 years ago
(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.
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

12 years ago
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.
(Reporter)

Comment 9

12 years ago
Created attachment 263090 [details] [diff] [review]
Patch with manual cropping

Incorporate Steffen's suggestion; the domain must always be visible. Otherwise nothing else has changed behaviour-wise.
Attachment #263014 - Attachment is obsolete: true
Attachment #263090 - Flags: review?(enndeakin)
Attachment #263014 - Flags: review?(enndeakin)
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
Attachment #263090 - Flags: review?(enndeakin)
(Reporter)

Comment 11

12 years ago
(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 on attachment 263013 [details]
Screenshot

You're wasting a lot of space here. Shouldn't the image and the text be arranged vertically?
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.
(Reporter)

Comment 14

11 years ago
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.
(Reporter)

Comment 15

11 years ago
Created attachment 263807 [details] [diff] [review]
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.
Attachment #263090 - Attachment is obsolete: true
(Reporter)

Updated

11 years ago
Summary: Tab previews should be part of the tab tooltip → Make tab tooltips more detailed (changed course from tab previews)
(Reporter)

Comment 16

11 years ago
Created attachment 265087 [details]
New screenshot
Attachment #263013 - Attachment is obsolete: true
Attachment #265087 - Flags: ui-review?(beltzner)
Attachment #263013 - Flags: ui-review?(beltzner)
(Reporter)

Comment 17

11 years ago
Created attachment 265088 [details] [diff] [review]
Updated patch
Attachment #263807 - Attachment is obsolete: true
(Reporter)

Comment 18

11 years ago
Comment on attachment 265088 [details] [diff] [review]
Updated patch

I'll just request review now while waiting for Beltzner's ui-r.
Attachment #265088 - Flags: review?(mano)
Comment on attachment 265088 [details] [diff] [review]
Updated patch

Please set the request once beltzner reviews the mockup.
Attachment #265088 - Flags: review?(mano)

Updated

11 years ago
Duplicate of this bug: 363599
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.
Attachment #265087 - Flags: ui-review?(beltzner) → ui-review+
Michael, time to request review? This needs an updated patch, though.
(Reporter)

Comment 23

11 years ago
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.
Back to the default assignee then. I'll leave this bug open, since there is a positive ui-review.
Assignee: ventnor.bugzilla → nobody
Status: ASSIGNED → NEW
Summary: Make tab tooltips more detailed (changed course from tab previews) → Make tab tooltips more detailed

Updated

11 years ago
Attachment #265088 - Attachment is obsolete: true
Duplicate of this bug: 444724
I think this is interesting, though I think tab preview is a better user experience.
Priority: -- → P4
Target Milestone: --- → Future

Updated

8 years ago
Duplicate of this bug: 615174
Created attachment 8339194 [details]
Rich tooltip example
Madahava posted this screenshot in bug 924888, in which he touches on this feature in light of Australis.
You need to log in before you can comment on or make changes to this bug.