Closed Bug 298950 Opened 16 years ago Closed 15 years ago

need copy in context menu

Categories

(SeaMonkey :: Help Viewer, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.8.1

People

(Reporter: tonglebeak, Assigned: martijn.martijn)

References

Details

(Keywords: fixed1.8.1)

Attachments

(2 files)

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
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.
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Hardware: PC → All
Target Milestone: --- → mozilla1.9alpha1
Attached patch patchSplinter Review
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.
Attachment #194205 - Flags: first-review?(jwalden+fxhelp)
Assignee: jwalden+fxhelp → martijn.martijn
Attachment #194205 - Flags: first-review?(jwalden+fxhelp) → first-review?(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 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.
Attachment #194205 - Flags: first-review?(steffen.wilberg) → first-review?(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.
Attached patch patch2Splinter Review
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.
Attachment #194205 - Attachment is obsolete: true
Attachment #194951 - Flags: first-review?(mconnor)
Attachment #194205 - Attachment is obsolete: false
Attachment #194205 - Flags: first-review?(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
Attachment #194951 - Flags: first-review?(mconnor) → first-review+
checked in by bz with the "/>" not on a separate line corrected.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
*** Bug 320399 has been marked as a duplicate of this bug. ***
We'll want to backport this for 2.0; I'm commenting so I don't forget about doing so.
Flags: blocking-firefox2?
Comment on attachment 194951 [details] [diff] [review]
patch2

The patch applies and works fine on the branch.
Attachment #194951 - Flags: approval-branch-1.8.1?(mconnor)
Attachment #194951 - Flags: approval-branch-1.8.1?(mconnor) → approval-branch-1.8.1+
not a blocker, but we'll take it
Flags: blocking-firefox2? → blocking-firefox2-
Fixed on 1.8 branch.
Keywords: fixed1.8.1
Target Milestone: mozilla1.9alpha1 → mozilla1.8.1
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?
(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.
Product: Toolkit → Seamonkey
You need to log in before you can comment on or make changes to this bug.