615483 - In the geolocation prompt the learn more hyperlink should flow with the question.

Status: NEW

(Firefox :: General, defect)

Description

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

Current the geolocation prompt has a hyperlink floating by itself in space:

Learn more...

This draws a lot of attention to it, diminishing the relative discoerabilty of the main action button.  Also, it isn't clear what you can learn more about until you go on to read the rest of the message.

We should visually group this hyperlink to the end of the question in the dialog:

Would you like to share your location with site.com? Learn more.

Also, the sentence does not need an ellipsis since no more user input is required, and the action is taken immediately (opening a new tab with a support document).

Comment 1

[:Margaret Leibovic]

What should we do about that fact that the text will likely wrap after the question? I feel like it would look awkward to have a question on one line, followed by a link on the next.

Also, this will require another string change :(

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Why string change? Can't we just move learnmore to be after popup-notification-description in the geolocation-notification binding?

Comment 3

[:Margaret Leibovic]

(In reply to comment #2)
> Why string change? Can't we just move learnmore to be after
> popup-notification-description in the geolocation-notification binding?

Getting rid of the ellipsis?

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Oh right. Well we can defer that part specifically for now, right? Might look a little odd but don't think it's a showstopper.

Comment 5

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

We can fix that later.  Most of the ellipsis in Firefox aren't actually correctly used.  When you click learn more the tab opens immediately, there isn't actually any additional user input required.

Comment 6

[:Margaret Leibovic]

Attachment: line break

With the patch from bug 615481 applied, the link appears on a new line below the main text. Is that acceptable, or should we do something to change that?

Comment 7

[:Margaret Leibovic]

Attachment: patch

Comment 8

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

Comment on attachment 494533 [details]
line break

turning down in favor of your idea of merging learn more into the action button (meh?)

Comment 9

[:Margaret Leibovic]

(In reply to comment #8)
> turning down in favor of your idea of merging learn more into the action button
> (meh?)

Gavin and I talked about that idea on IRC yesterday, and we decided it would be simpler to just leave the link. A patch to make it into a menu item would have to modify the popup notification API to allow for secondary actions that don't remove the notification, and we would definitely need a string change to give the menu item an access key.

What do you think? And if we do keep the link, what do you think of that screenshot I attached?

Comment 10

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

Comment on attachment 494533 [details]
line break

yeah, that's fine.  Let's fix this after Firefox 4, and introduce learn more options for some of the other notifications (indexedDB, protocol registration, etc).

Comment 11

[Dão Gottwald [:dao]]

Comment on attachment 494534 [details] [diff] [review]
patch

This looks like it regresses bug 615481.

Comment 12

[:Margaret Leibovic]

(In reply to comment #11)
> Comment on attachment 494534 [details] [diff] [review]
> patch
> 
> This looks like it regresses bug 615481.

In what way?

Comment 13

[Dão Gottwald [:dao]]

Comment on attachment 494534 [details] [diff] [review]
patch

Here:

>-        <xul:description class="popup-notification-description"
>-                         xbl:inherits="xbl:text=label"/>
>+      <xul:vbox flex="1">
>+        <xul:description class="popup-notification-description">
>+          <xul:label xbl:inherits="value=label"/>

<label value="..."/> won't wrap, as I understand it.

Comment 14

[:Margaret Leibovic]

Attachment: patch v2

Ah, you're right. This patch has text that wraps. I couldn't get the link to appear on the same link as the description text using two labels, so instead I decided to insert the link inside the description after the text. To make a space between the text and the link, I set a -moz-margin-start rule on the link, but that makes the link look misaligned if it falls on its own line. Any ideas for a solution to that?

Comment 15

[Dão Gottwald [:dao]]

It would usually wrap right before the link, wouldn't it? So just always put the link on its own line?

Comment 16

[:Margaret Leibovic]

I think it probably depends on whatever system font size/resolution the user has. For example, the text wraps in the screenshot from bug 617270: https://bug617270.bugzilla.mozilla.org/attachment.cgi?id=495781. In that case, it would be nice to have the link appear at the end of the text.

Comment 17

[:Margaret Leibovic]

Attachment: patch v3

Using a spacer instead of a margin solves the text wrap problem.

Comment 18

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 496020 [details] [diff] [review]
patch v3

Instead of dynamically creating the label/spacer, couldn't you have them hardcoded in the <description>, and just set their values?

Comment 19

[:Margaret Leibovic]

Attachment: patch v3 (unbitrotted)

(In reply to comment #18)
> Comment on attachment 496020 [details] [diff] [review]
> patch v3
> 
> Instead of dynamically creating the label/spacer, couldn't you have them
> hardcoded in the <description>, and just set their values?

The problem with that is the xbl:inherits="xbl:text=label" will insert the description text after the spacer/label instead of before them.

Comment 20

[Dão Gottwald [:dao]]

Attachment: patch

Comment 21

[:Gavin Sharp [email: [e-mail]]]

(In reply to Alex Faaborg [:faaborg] (Firefox UX) from comment #0)
> We should visually group this hyperlink to the end of the question in the
> dialog:
> 
> Would you like to share your location with site.com? Learn more.
> 
> Also, the sentence does not need an ellipsis since no more user input is
> required, and the action is taken immediately (opening a new tab with a
> support document).

That's not really what this patch implements, AFAICT. I think the ui-r+ed screenshot was ui-r+ed only as a compromise for Firefox 4 (see comment 10).

Comment 22

[Dão Gottwald [:dao]]

The current patch is all I can offer, I can't spend more time on this now.

Comment 23

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 695306 [details] [diff] [review]
patch

This is a net improvement in that it gets rid of the ugly binding overriding, but doesn't fix this bug as filed. Land it in a new bug?