Daniel Holbert [:dholbert] Reporter
	Description • 14 years ago

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.

	Daniel Holbert [:dholbert] Reporter
	Comment 1 • 14 years ago

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.

	gadjo
	Comment 3 • 14 years ago

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"?

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 4 • 14 years ago

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

	Nochum Sossonko [:Natch]
	Comment 5 • 14 years ago

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

	Jared Wein [:jaws] (please needinfo? me) Assignee
	Comment 6 • 13 years ago

I'll take a stab at this bug

	Jared Wein [:jaws] (please needinfo? me) Assignee
	Comment 7 • 13 years ago

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.

	Jared Wein [:jaws] (please needinfo? me) Assignee
	Comment 8 • 13 years ago

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

	Jared Wein [:jaws] (please needinfo? me) Assignee
	Comment 9 • 13 years ago

r=gavin

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 10 • 13 years ago

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.)

	Nochum Sossonko [:Natch]
	Comment 11 • 13 years ago

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)

	Jared Wein [:jaws] (please needinfo? me) Assignee
	Comment 12 • 13 years ago

(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?

	Nochum Sossonko [:Natch]
	Comment 13 • 13 years ago

(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.

	Jared Wein [:jaws] (please needinfo? me) Assignee
	Comment 14 • 13 years ago

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

	Jared Wein [:jaws] (please needinfo? me) Assignee
	Comment 15 • 13 years ago

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".

	:Margaret Leibovic
	Comment 16 • 13 years ago

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.

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 17 • 13 years ago

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.

	Jared Wein [:jaws] (please needinfo? me) Assignee
	Comment 18 • 13 years ago

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?

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 19 • 13 years ago

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

Certainly.

	Jared Wein [:jaws] (please needinfo? me) Assignee
	Comment 20 • 13 years ago

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.

	Justin Dolske [:Dolske]
	Comment 21 • 13 years ago

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

	Vlad [QA]
	Comment 22 • 13 years ago

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