298950 - need copy in context menu

Status: RESOLVED FIXED

(SeaMonkey :: Help Viewer, enhancement)

Description

[Aaron Slunt]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050620 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050620 Firefox/1.0+

Quite simple: when you want to copy text from help, the only way to copy is
ctrl+c. Why not go ahead and add copy to the context menu as well? Not everyone
knows the copy keyboard shortcut.

Reproducible: Always

Comment 1

[Jeff Walden (gone starting June 8)]

This seems like it would be worthwhile, but because Copy only shows up when
there's actually something to copy, it'd be a pain to code in the remaining time
we have before 1.1.  Look for this after then, but definitely not for 1.1.

Comment 2

[Martijn Wargers (dead)]

Attachment: patch

Like this?
This makes the copy menu-item disabled when there is nothing to copy (which is,
what I (think) prefer).
I can change it to be collapsed, if you prefer.

Comment 3

[Steffen Wilberg]

Please request review from mconnor.
But while I'm here, I think it's correct to disable and not collapse "copy" when
nothing is selected, because "select all" is shown all the time. The completely
different selection-context menu of the browser doesn't make sense in the Help
Viewer because it doesn't offer Search Web or View Selection Source.

Comment 4

[Martijn Wargers (dead)]

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

Ok, thanks.
I actually would like to have those context menu items (search web.., view
selection source) too, but that's probably too much overkill for a help viewer.

Comment 5

[Mike Connor [:mconnor]]

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

Context menus should, as a general rule, only show things relevant to the
current context.  If there isn't a selection, don't show Copy, period.

This is definitely too late for 1.5, since we're in slushyfreeze for l10n, but
it'll be good for 2.0 once it actually collapses instead of disables.

Comment 6

[Martijn Wargers (dead)]

Attachment: patch2

Like this?
Personally I prefer to see the copy item disabled.
The help context menu is small and only one item in it is changing.
The context menu in the content area is a lot bigger, there I think it is a
good idea to hide/unhide menu-items, otherwise the context menu would become
too big. Also, multiple context menu-items are changing when something is
selected.

Comment 7

[Mike Connor [:mconnor]]

Comment on attachment 194951 [details] [diff] [review]
patch2


>+      <menuitem id="context-copy"
>+                label="&copyCmd.label;"
>+                accesskey="&copyCmd.accesskey;"
>+                command="cmd_copy"
>+                disabled="true"
>+                />

this shouldn't be orphaned like this

Comment 8

[Martijn Wargers (dead)]

checked in by bz with the "/>" not on a separate line corrected.

Comment 9

[Steffen Wilberg]

*** Bug 320399 has been marked as a duplicate of this bug. ***

Comment 10

[Jeff Walden (gone starting June 8)]

We'll want to backport this for 2.0; I'm commenting so I don't forget about doing so.

Comment 11

[Steffen Wilberg]

Comment on attachment 194951 [details] [diff] [review]
patch2

The patch applies and works fine on the branch.

Comment 12

[Mike Connor [:mconnor]]

not a blocker, but we'll take it

Comment 13

[Steffen Wilberg]

Fixed on 1.8 branch.

Comment 14

[Steffen Wilberg]

Comment on attachment 194951 [details] [diff] [review]
patch2

>Index: toolkit/themes/winstripe/help/help.css
>+#context-copy[disabled="true"] {
>+  visibility: collapse;
>+}

Umm, shouldn't this be added to Pinstripe's help.css as well?

Comment 15

[[e-mail]]

(In reply to comment #14)
>(From update of attachment 194951 [details] [diff] [review] [edit])
>>Index: toolkit/themes/winstripe/help/help.css
>>+#context-copy[disabled="true"] {
>>+  visibility: collapse;
>>+}
>Umm, shouldn't this be added to Pinstripe's help.css as well?
Actually it should never have been added to Winstripe's help.css - visibility: collapse doesn't remove keyboard access to the menuitem.