598169 - The new "About" dialog should allow version-string to be selectable (e.g. for Copy/Paste into bug reports)

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Daniel Holbert [:dholbert]]

STEPS TO REPRODUCE:
 1. Help | About
 2. Try to select your version-string, right above "Check for Updates" button.
    The text is something like: 4.0b7pre (released MM-DD-YYYY)

ACTUAL RESULTS: That text isn't selectable.

EXPECTED RESULTS: That text should be selectable, so users can e.g. copy/paste it into bug reports or IRC conversations about issues they're experiencing.

Comment 1

[Daniel Holbert [:dholbert]]

FWIW: I'd expect this text to be selectable because in the past, we've always told users to copy & paste version information from Help|About when filing/describing bugs.

However, as Margaret points out in IRC, this information is also available (and copyable) in about:support now, too.  So, maybe we should just be ignoring Help|About for the purposes of version-identification, from now on.

IMHO, it would still be handy to have this text be selectable, so as not to break power-user's habits.  But, probably not a blocker & possibly a WONTFIX.

Comment 3

[gadjo]

I don't think much users (especially the less-technical) know about "about:support". Maybe it's a good idea to add this link to "About" window? For example "Show details" or "More information" next to the version string? Or make a version string as a link to "about:support"?

Comment 4

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

It's reachable from the Help menu ("Troubleshooting Information")

Comment 5

[Nochum Sossonko [:Natch]]

I think all the text should be selectable in the about window, why is it not?

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

I'll take a stab at this bug

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

I've spent some time on this bug and have a fix for it but the fix makes the about dialog quite janky.

1. In order to make the version selectable, I changed the XUL element to a textbox. Selections in textboxes can't escape the element, so selecting all the text on the dialog in one movement isn't possible.
2. After a selection is made, it is somewhat hard to cancel the selection.
3. If a user is able to cancel the selection, or clicks on the version string without making a section, then a carat will stay blinking in the version string. This feels very janky.

I propose that we mark this bug as WONTFIX but add a new bug for Bugzilla to include a link to about:support since that is likely the majority of cases where people are trying to grab the version number.

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch for bug 598169

This patch allows a user to select and copy/paste the version string from the about dialog.

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

r=gavin

Comment 10

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

As we discussed on IRC, it isn't necessary to make this a textbox. You can just change the label in aboutDialog.xul from:
#expand <label id="version" value="__MOZ_APP_VERSION__"/>
to:
#expand <label id="version">__MOZ_APP_VERSION__</label>

and then set -moz-user-select: text; (and potentially -moz-user-focus: normal;) to make it selectable.

(The change required for the label is a weird quirk of the XUL layout code, some details of which are in bug 438517 comment 3 - depending on the markup you end up with different kind of layout objects, and only one of them properly supports text selection.)

Comment 11

[Nochum Sossonko [:Natch]]

Comment on attachment 539393 [details] [diff] [review]
Patch for bug 598169

Review of attachment 539393 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/aboutDialog.css
@@ +18,5 @@
>    padding: 15px 10px 0;
>  }
>  
>  #version {
> +  margin-top: 10px !important;

Why is this necessary? (curious)

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to comment #11)
> Comment on attachment 539393 [details] [diff] [review] [review]
> Patch for bug 598169
> 
> Review of attachment 539393 [details] [diff] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/base/content/aboutDialog.css
> @@ +18,5 @@
> >    padding: 15px 10px 0;
> >  }
> >  
> >  #version {
> > +  margin-top: 10px !important;
> 
> Why is this necessary? (curious)

This was because |class="plain"| added |margin-top: 0pt|. I suppose we may be able to get rid of the |!important| if we use a more specific selector?

Comment 13

[Nochum Sossonko [:Natch]]

(In reply to comment #12)
> This was because |class="plain"| added |margin-top: 0pt|. I suppose we may
> be able to get rid of the |!important| if we use a more specific selector?

#version should override any class styles, ids are more specific than classes by definition.

Comment 14

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch for bug 598169 - version 2

Fixed the issues brought up. Switched back to a label.

Comment 15

[Jared Wein [:jaws] (please needinfo? me)]

I forgot to mention something. While this patch allows a user to select and copy the text, there is no right-click context menu with this patch. It seems that we would want that, and that it should only include "Select All" and "Copy".

Comment 16

[:Margaret Leibovic]

Comment on attachment 539565 [details] [diff] [review]
Patch for bug 598169 - version 2

This patch doesn't really need ui-review, since it's a very small change that doesn't really change the UI. Generally, we reserve ui-review for members of the UX team to review more significant or potentially controversial changes.

Comment 17

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

Comment on attachment 539565 [details] [diff] [review]
Patch for bug 598169 - version 2

I don't think we need to worry about a context menu.

You should probably also add |-moz-user-focus: normal;| and |cursor: text;| to match the commonDialog behavior. r=me with that change.

Comment 18

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch for bug 598169 - version 3

Added the two CSS rules. I'll send this to a try server, and then pending the results can you land this for me gavin?

Comment 19

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

Comment on attachment 539592 [details] [diff] [review]
Patch for bug 598169 - version 3

Certainly.

Comment 20

[Jared Wein [:jaws] (please needinfo? me)]

The try server passed with 1 warning in the JetPack tests.

http://tbpl.mozilla.org/?tree=Try&rev=01e4076c8105

Please land this if you don't think the warning is related to my modifications. Thanks Gavin.

Comment 21

[Justin Dolske [:Dolske]]

Pushed http://hg.mozilla.org/mozilla-central/rev/c0b984e8ade0

Comment 22

[Vlad [QA]]

Setting resolution to Verified Fixed on Mozilla/5.0 (Windows NT 6.1; rv:7.0a1) Gecko/20110619 Firefox/7.0a1.

The text is selectable but the copy and paste is made only from the keyboard with ctrl+c/v