Closed Bug 312930 Opened 15 years ago Closed 15 years ago

rich text editor doesn't have context menu items for cut/copy/paste/spellcheck

Categories

(Firefox :: Menus, enhancement)

x86
Windows XP
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: nrlz, Assigned: martijn.martijn)

References

Details

(Keywords: fixed1.8.1)

Attachments

(4 files, 7 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.12) Gecko/20050915 Firefox/1.0.7
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.12) Gecko/20050915 Firefox/1.0.7

When right-clicking on a Rich Text Editor (e.g., in a Gmail compose window), the
context-menu does not have the options for cut, copy and paste. The cut, copy
and paste options, do however, appear in the "Edit" menubar and work as expected.

Probably involves some additional detection here:
http://lxr.mozilla.org/mozilla/source/browser/base/content/browser.js#4181

Reproducible: Always

Steps to Reproduce:
1. Go to "www.gmail.com"
2. Click "compose"
3. Make sure "rich formatting" is enabled
4. Right-click on compose area

Actual Results:  
No see Cut/Copy/Paste.

Expected Results:  
See Cut/Copy/Paste.
This is because it's not an element that needs edit commands in its context menu.
Status: UNCONFIRMED → RESOLVED
Closed: 15 years ago
Resolution: --- → INVALID
Well IE has this feature so I just thought it would be nice if Firefox had it
too.
I noticed that I can't paste into a gmail rich text compose window using a right click menu option.   Ctrl-V works.  I tried in safe mode and with a new profile.  When right clicking, with something in the clipboard, a paste option should be available.  I think this bug should be reopened.  

(In reply to comment #3)
> I noticed that I can't paste into a gmail rich text compose window using a

This is my build: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051029 Firefox/1.5 ID:2005102903
Since my bug is related to paste this may need a new bug report?  I was trying to avoid posting a duplicate bug.
*** Bug 319890 has been marked as a duplicate of this bug. ***
*** Bug 314577 has been marked as a duplicate of this bug. ***
Status: RESOLVED → UNCONFIRMED
Resolution: INVALID → ---
Attached patch patch1 (obsolete) — Splinter Review
This fixes it. With this patch, you also get the "This Frame >" menu-item, which is not what IE6 is doing, but I think it makes more sense to keep it in.
Attachment #205900 - Flags: review?(mconnor)
Assignee: nobody → martijn.martijn
Status: UNCONFIRMED → NEW
Ever confirmed: true
Not sure how to fix SeaMonkey yet. Anybody have ideas?
Status: NEW → ASSIGNED
*** Bug 320400 has been marked as a duplicate of this bug. ***
Attached file Testcase
I added testcase, here are steps for reproduce:

1. Open testcase
2. Write any text to editable area
3. Select text and use Copy function from context menu
4. Unselect text and open contex menu

Actual Results:  
There item "Cut" in context menu is missing , when is text selected.
There "Paste" in context menu  is missing, when is text unselected.

Expected Results:  
There will be item "Cut" in context menu, when is text selected.
There will be item "Paste" in context menu, when is text unselected.
Comment on attachment 205900 [details] [diff] [review]
patch1

This isn't enough. I need to disable this.onlink, this.onimage, etc, otherwise you get weird context menus when right-clicking on images/links, etc.
New patch hopefully tomorrow.
Attachment #205900 - Flags: review?(mconnor)
Attached patch patchv2 (obsolete) — Splinter Review
This fixes it.
I've also made it work on designmode documents that aren't in iframes.
I've disabled this.inframe, on second thought I think it's not useful.
I've kept the View selection Source, though, as I think that one is still really useful here.
Attachment #205900 - Attachment is obsolete: true
Attachment #206666 - Flags: review?(mconnor)
Thanks. This is/was really an annoying bug.
Hi, I write because I need help with the bug 312930 (copy/paste in editable frame)
The patch is the complete file, or is to add, and where I add this code?, else attach the complete file and where to place.
Another thing, I have a problem similar to this bug, when I copy image with the contextual menu, and paste into the editable frame, this not paste the image, only paste the data content of image from the hard disk (c:\documents and...\temp...\image.jpg)
The only way to paste image in the editable frame is:
1 View Image (contextual menu)
2 CTRL+A
3 CTRL+C or copy in the contextual menu
4 CTRL+V in the gmail compose

I try use the "Copy Image" extension and modify the code where say "cmd_copyImageContents" and replace with "cmd_copy", but don't works, the copy function still disable when the image is not selected.
Copy Image Forum
http://www.mozilla-center.de/forum/posting.php?mode=quote&p=1798&sid=094d911e46a56f4f3ee1c3ae3b5d8a86
Copy Image File
http://www.dsius.de/copyimage.zip

I don't know if is the same bug, but is annoying and I need your opinion if is another bug to report or not.
This problem begin in Mozilla Firefox 1.0.x to 1.5.0.1, in Windows XP and 2000, I don't know in Win9x
Please helpme.


Mozilla/5.0 (Windows; U; Windows NT 5.1; es-AR; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1
The big problem with this is that I have doen everything that the FAQ section has told me to, and Cut/Past still don't work.  Grr....
Attached patch patchv3 (obsolete) — Splinter Review
This is like patchv2, but now also with the spell-checking code. This seems to work just fine for designMode iframes.
Attachment #223612 - Flags: review?(brettw)
Comment on attachment 223612 [details] [diff] [review]
patchv3

What if they click on an image in an editable document? It looks like you set onImage to false in this code, which would disable the normal image editing commands.
Attached patch patchv4 (obsolete) — Splinter Review
So like this? I had to add some code to not get a double seperator when right clicking on an image in designMode.
Attachment #223612 - Attachment is obsolete: true
Attachment #223623 - Flags: review?(brettw)
Attachment #223612 - Flags: review?(brettw)
Attachment #223623 - Attachment is obsolete: true
Attachment #223623 - Flags: review?(brettw)
Attached patch patchv4 (obsolete) — Splinter Review
Forgot something in the previous patch.
Attachment #223624 - Flags: review?(brettw)
I still don't understand this:
+            this.isDesignMode      = true;
+            this.onTextInput       = true;
+            this.onKeywordField    = false;
+            this.onLink            = false;
+            this.onMailtoLink      = false;
+            this.onSaveableLink    = false;
+            this.onMetaDataItem    = false;
+            this.onMathML          = false;
+            this.inFrame           = false;
+            this.hasBGImage        = false;

It seems almost all of these things could appear in a designmode document, and then you wouldn't get the appropriate menuitems for it. Am I missing something?
Blocks: 333038
Flags: blocking-firefox2?
(In reply to comment #22)
> It seems almost all of these things could appear in a designmode document, and
> then you wouldn't get the appropriate menuitems for it. Am I missing something?

No, you're right. I thought it would be a good idea at the time to not clutter up the context menu when in designMode.
I'll update the patch with the necessary changes.
Flags: blocking-firefox2? → blocking-firefox2+
Comment on attachment 206666 [details] [diff] [review]
patchv2

from the look of the bug, this is obsoleted by later patches, please re-request if I've misread!
Attachment #206666 - Flags: review?(mconnor)
Attachment #206666 - Attachment is obsolete: true
I'm still not convinced that disabling the "regular" commands is correct in design mode. For example, if I right-click on a link or an image in designmode, I still might expect that the commands work. This is the behavior right now, and I'm worried about changing it.

Beltzner, do you have any comments on this?
I just tested IE, and it does it your way (only editing commands in design mode). This might be best. We should make a conscious decision on the proper behavior, however.
Comment on attachment 223624 [details] [diff] [review]
patchv4

This change looks like it got messed up:
>-              this.InlineSpellCheckerUI.init(this.inputField);
>+              this.InlineSpellCheckerUI.init(this.inputFieldthis.target.QueryInterface(Components.interfaces.nsIDOMNSEditableElement).editor);
Attachment #223624 - Flags: review?(brettw) → review-
Ok, I'll wait on what Mike Beltzner has to say. Indeed, I messed that up in my last patch, I'll fix it as soon as the desired ui is known.
Keywords: uiwanted
The user won't understand that a textbox is in design mode or not, and instead will just interpret it as a textbox with some added controls (which they'll probably credit to the site designers). We should be offering them the same sort of cut/copy/paste/save as/ options that we would in any other text editing box.

If we *can* get controls for saving an image that appears in designmode, I think that would be a good thing. I don't know if we want to go hog-wild for all links and mailto's, though, since that would be an inconsistent experience with the context menu of a regular textbox.

The "This Frame >" stuff should be suppressed unless there's a powerful use case to preserve it.
Might not want to check this in the 1.8.1 branch, unless bug 287707 is also fixed.
Depends on: 287707
After some discussions in IRC, we decided it will be best NOT to suppress link context-menu commands in design mode. This is because there is not any other way to follow links (clicking on them sets the cursor).
No longer depends on: 287707
(In reply to comment #31)
> Might not want to check this in the 1.8.1 branch, unless bug 287707 is also
> fixed.

It seems to me these bugs are not related. This is on improving the context menu, that one is on fixing functionality issues.
For the record, we were on the fence about "View Selection Source". Some people felt it would be useful. "View Source" on the other hand will show you the page before you started editing, so it not very useful.

If it seems some functionality is missing, we can always add them back later, but we should try to have as few things in the context menu as possible.
Also, I don't think we will do image commands at this point either. We can revisit that as well. For the typical use-cases of designmode (webmail), view image is not useful.
I'll come up with an updated patch as soon as bug 340320 is fixed.

> (In reply to comment #31)
> It seems to me these bugs are not related. This is on improving the context
> menu, that one is on fixing functionality issues.
Bug 287707 causes various things to remain in designMode mode when visiting another page. when this bug is fixed, it means that the context menu also stays in designMode.
(In reply to comment #35)
> Bug 287707 causes various things to remain in designMode mode when visiting
> another page. when this bug is fixed, it means that the context menu also stays
> in designMode.

We need this patch for Firefox 2 due to the spellchecking. Bug 287707 is targeted for Firefox 3, this patch is blocking Firefox 2. I suppose we could nominate that bug for blocking Firefox 2, but I'm not sure if it will get fixed. That bug is not typically encountered in the usecases for designmode so I would argue is not a priority.

I'll check in bug 340320 later today.
Summary: rich text editor doesn't have context menu items for cut/copy/paste → rich text editor doesn't have context menu items for cut/copy/paste/spellcheck
(In reply to comment #36)
> I'll come up with an updated patch as soon as bug 340320 is fixed.

I just landed that bug.
Attached patch patch5 (obsolete) — Splinter Review
Ok, thanks, here it is. so most commands are disabled, but I didn't disable "View Selection Source" and "Search Google for ...", because I like to have those. In fact, I would love to have a "Search Google for ..." for input fields.
The rest is disabled, because that is what is decided. Or isn't (after reading previous comments, I'm not too sure anymore).
Attachment #223624 - Attachment is obsolete: true
Attachment #226258 - Flags: review?(brettw)
Attached patch patch5 (obsolete) — Splinter Review
Sorry, previous patch contained an error.
Attachment #226258 - Attachment is obsolete: true
Attachment #226259 - Flags: review?(brettw)
Attachment #226258 - Flags: review?(brettw)
Comment on attachment 226259 [details] [diff] [review]
patch5

As per comment 32, can you please have the link context menu commands work if it is on a link? Otherwise this looks good.
Attachment #226259 - Flags: review?(brettw) → review-
Attached patch patch6Splinter Review
Ok, done. I also let the "context-selectall" menu-item appear when a context menu appears on a link in designMode. Otherwise, if you don't like that, then I'll have to remove a separator in that case, probably the "context-sep-paste" separator.
Attachment #226259 - Attachment is obsolete: true
Attachment #226370 - Flags: review?(brettw)
Comment on attachment 226370 [details] [diff] [review]
patch6

This looks fine. We can reevaluate which things to show when we start using it regularly.
Attachment #226370 - Flags: review?(brettw) → review+
No need for sr, right?
Keywords: uiwanted
(In reply to comment #44)
> No need for sr, right?

No.
Comment on attachment 226370 [details] [diff] [review]
patch6

And I guess this needs approval1.8.1
Attachment #226370 - Flags: approval1.8.1?
(In reply to comment #45)
> (In reply to comment #44)
> > No need for sr, right?
> 
> No.

I mean "correct" you don't need SR. It should bake on trunk before we check into branch, so we should check it in ASAP.
Checking in browser/base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.648; previous revision: 1.647
done
Checking in toolkit/content/inlineSpellCheckUI.js;
/cvsroot/mozilla/toolkit/content/inlineSpellCheckUI.js,v  <--  inlineSpellCheckU
I.js
new revision: 1.7; previous revision: 1.6
done
Checking in toolkit/content/widgets/textbox.xml;
/cvsroot/mozilla/toolkit/content/widgets/textbox.xml,v  <--  textbox.xml
new revision: 1.34; previous revision: 1.33
done
Status: ASSIGNED → RESOLVED
Closed: 15 years ago15 years ago
Resolution: --- → FIXED
Comment on attachment 226370 [details] [diff] [review]
patch6

Please request approval after another day or two to ensure this bakes well.
Attachment #226370 - Flags: approval1.8.1?
Attachment #226370 - Flags: approval1.8.1?
Comment on attachment 226370 [details] [diff] [review]
patch6

a=darin on behalf of drivers (please land on the MOZILLA_1_8_BRANCH and add the fixed1.8.1 keyword to the bug)
Attachment #226370 - Flags: approval1.8.1? → approval1.8.1+
Checking in browser/base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.479.2.153; previous revision: 1.479.2.152
done
Checking in toolkit/content/inlineSpellCheckUI.js;
/cvsroot/mozilla/toolkit/content/inlineSpellCheckUI.js,v  <--  inlineSpellCheckU
I.js
new revision: 1.2.10.6; previous revision: 1.2.10.5
done
Checking in toolkit/content/widgets/textbox.xml;
/cvsroot/mozilla/toolkit/content/widgets/textbox.xml,v  <--  textbox.xml
new revision: 1.21.4.9; previous revision: 1.21.4.8
done

Checked into 1.8.1 branch.
Keywords: fixed1.8.1
You need to log in before you can comment on or make changes to this bug.