Closed Bug 585875 Opened 9 years ago Closed 9 years ago

Extend Fennec's copy/paste support

Categories

(Firefox for Android Graveyard :: General, defect)

x86
Linux
defect
Not set

Tracking

(fennec2.0b3+)

VERIFIED FIXED
Tracking Status
fennec 2.0b3+ ---

People

(Reporter: antti.i.jarvelin, Assigned: mbrubeck)

References

Details

(Whiteboard: [has-patch] [strings])

Attachments

(2 files, 2 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.8) Gecko/20100723 Ubuntu/10.04 (lucid) Firefox/3.6.8
Build Identifier: Mozilla/5.0 (X11; Linux armv7I; rv:2.0b3pre) Gecko/20100727 Firefox/4.0b3pre Fennec/2.0a1pre

Extending the copy/paste support includes: 
1. Pasting text into html text fields 
2. Paste clipboard contents into editable html elements
3. Copy functionality for text & html contents of web pages
4. Cut functionality for text fields and content editable html elements
5. The user should be able to trigger the above actions using context sensitive
menu or keyboard
shortcuts if keyboard is available
6. Users should be able to copy link locations into clipboard with context
sensitive menus.
7. Users should be able to copy image locations into clipboard with context
sensitive menus.




Reproducible: Always
Opening a new bug for Fennec because Bug 582912 is set RESOLVED FIXED, but the patch for Fennec is not yet processed.
Attached patch Fix Proposal for fennec (obsolete) — Splinter Review
This patch extends the copy/paste support of fennec.
Attachment #464340 - Flags: review?(mark.finkle)
Status: UNCONFIRMED → NEW
tracking-fennec: --- → ?
Ever confirmed: true
tracking-fennec: ? → 2.0+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 582912
Not a dupe. See comment 1.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Blocks: 590349
No longer blocks: 590349
Depends on: 582912, 590349
Blocks: 584225
I think this bug is very dependent on some form of text selection capability.
Copy/Paste alone isn't much good without selecting text. I am worried that we
won't have enough time in Fennec 2 for text selection.
The textselection can be done by extensions, just because there is no possibility to select text, does not mean we need to keep that broken.
I don't know if this is known, but you can do select text, but you can select text by tapping on the text to select, then shift-arrow right or left.
As a first step, we could imitate Android and have "select all" and "copy all" and "cut all" in the context menu for text fields (when there is no selection already).
What are the plans with this patch? Is it going to be integrated any time soon?
I like Matt's suggestion in comment 8 - it gives us the functionality that I think most people want (copying an entire URL, selecting all to delete quickly) without requiring figuring out a full text selection system.

On that note, how useful is "cut" if you have both "copy all" (for copying all) and "select all" (or deleting the text).  I mean, it has some use, but we're also trying to set a high bar for inclusion here, or the menus will get too tall.  I think that many of the menus on android are already too long.
Paste:
- any time a field is focussed and there is something in clipboard

Copy:
- when there is a selection

Copy all:
- on a field when there is no selection

Select all:
- when a field is focussed

No cut.
Duplicate of this bug: 449895
Long-press on Android doesn't seem to do anything. This problematic as those of us without hardware keyboards (like the HTC EVO) can't copy/paste form text in Fennec.
Duplicate of this bug: 602689
I started implementing context menu commands for XUL textboxes.  Some of the commands (paste, select all) do not actually work yet.

This patch uses ContextHelper.showPopup to display the context menu, but after trying this out, I wonder if we should try to integrate into the platform more (using the "context" attribute, and menupopups styled like Fennec dialogs).  Any thoughts?
Assignee: nobody → mbrubeck
Status: REOPENED → ASSIGNED
Attachment #482636 - Flags: feedback?(mark.finkle)
Comment on attachment 482636 [details] [diff] [review]
Context menu for chrome textboxes (WIP)

I like the approach. We should think about making an XBL for <contextmenuitem> or similar. The <richlistitem> approach is getting laborious and messy.
Attachment #482636 - Flags: feedback?(mark.finkle) → feedback+
I can't believe Fennec is at version 4.0pre on Android and still doesn't have copy/paste!!!
tracking-fennec: 2.0+ → 2.0b3+
Whiteboard: [has-patch]
Comment on attachment 464340 [details] [diff] [review]
Fix Proposal for fennec

Let's take a smaller version.
Attachment #464340 - Flags: review?(mark.finkle) → review-
Whiteboard: [has-patch] → [has-patch] [strings]
Matt - just checking on the status
Not a lot of progress since the WIP patch. I spend about half of yesterday working on it, and will continue it all today.  I hope to have it ready tomorrow morning.  I'll check in with another update later today.
Incidentally, the copy/paste functionality for the full-screen text-editor IME (the one typically seen in landscape mode) should be fixed by my patch for bug 601204 (patch coming soon)
Fully working version of the previous patch.  This is still for chrome only.
Attachment #482636 - Attachment is obsolete: true
Attachment #489842 - Flags: review?(mark.finkle)
Comment on attachment 489842 [details] [diff] [review]
Context menu for chrome textboxes

Seems to work ok. On desktop, sometimes the long tap would not bring up the "paste" menu when I know something was on the clipboard. A second try would work though.
Attachment #489842 - Flags: review?(mark.finkle) → review+
Pushed: http://hg.mozilla.org/mobile-browser/rev/e634cc4a8c35

Resolving this bug; we can file a new one for copy/paste in content.

(In reply to comment #24)
> Seems to work ok. On desktop, sometimes the long tap would not bring up the
> "paste" menu when I know something was on the clipboard. A second try would
> work though.

I think the two-tap-to-focus thing in landscape mode may affect the context menu too.  We might need to do something about this in a followup.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
After trying this on device (Android), it needs more work.  The content menu is very hard to bring up, even in cases where it works fine on Linux.  Reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to comment #26)
> After trying this on device (Android), it needs more work.  The content menu is
> very hard to bring up, even in cases where it works fine on Linux.

I think this is because the ui.click_hold_context_menus pref does not allow for any mouse movement - if you move your finger by even 1px while pressing on the textbox, then the context menu does not open.  We might want to use our own TapLong event instead of click_hold_context_menus.
This patch adds a TapLong handler, to make long press usable on touchscreen.

It also fixes a problem on Android where the virtual keyboard would slide in and out when starting and finishing long tap events.  It removes the blurFocusedElement call that was added by bug 599362.  Instead, it adds an event listener that hides the context menu on keydown.  (This matches desktop Firefox behavior.)

The context menu still interacts badly with the Android virtual keyboard in landscape mode.
Attachment #464340 - Attachment is obsolete: true
Attachment #489910 - Flags: review?(mark.finkle)
Comment on attachment 489910 [details] [diff] [review]
part 2: Fix long-tap behavior for Android

>diff -r e634cc4a8c35 chrome/content/bindings.xml

>+
>+      <handler event="TapLong" phase="capturing">
>+        <![CDATA[
>+          let box = this.inputField.parentNode;
>+          box.showContextMenu(this, true);
>+        ]]>
>+      </handler>

I suppose this is easier than needing to do all kinds of gymnastics to get the new <textbox> binding to handle this via "xbl extending"

>diff -r e634cc4a8c35 chrome/content/browser-ui.js

>   handleEvent: function handleEvent(aEvent) {

>+      case "keypress":
>+        aEvent.stopPropagation();
>+        aEvent.preventDefault();
>+        if (aEvent.keyCode != aEvent.DOM_VK_ESCAPE)
>+          this.hide();

I'm happy we no longer blur the element. Closing the context menu leaves the focus where it was. How about adding a simple comment saying we do this so the active textbox doesn't accept keystrokes while the context menu is active.
Attachment #489910 - Flags: review?(mark.finkle) → review+
Forgot to ask. Do we still need "ui.click_hold_context_menus = true" ?
(In reply to comment #30)
> Forgot to ask. Do we still need "ui.click_hold_context_menus = true" ?

Yes - it looks like it's still useful for things like awesomescreen rows, which don't seem to have the problem with mousemoves.  That must be a textbox thing.

(In reply to comment #29)
> I suppose this is easier than needing to do all kinds of gymnastics to get the
> new <textbox> binding to handle this via "xbl extending"

Yes, exactly.  I imagine that's the same reason that toolkit puts its contextmenu behavior in the input-box binding rather than the textbox itself.

> I'm happy we no longer blur the element. Closing the context menu leaves the
> focus where it was. How about adding a simple comment saying we do this so the
> active textbox doesn't accept keystrokes while the context menu is active.

Done, and pushed:
http://hg.mozilla.org/mobile-browser/rev/f29b43d1a6d7

Resolving this bug; will file followups for some remaining polish issues.
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Depends on: 611674
Blocks: 611736
Blocks: 611730
Blocks: 611741
Filed followup bugs:
Bug 611741 - Implement copy/paste for text fields in web content
Bug 611730 - urlbar context menu and landscape keyboard both appear on long tap
Bug 611736 - Long-tap in textbox causes a click, which changes selection
\o/ verified FIXED On build:

Mozilla/5.0 (Maemo; Linux armv71; rv:2.0b8pre) Gecko/20101115 Namoroka/4.0b8pre Fennec/4.0b3pre

and

Mozilla/5.0 (Android; Linux armv71; rv:2.0b8pre) Gecko/20101115 Namoroka/4.0b8pre Fennec/4.0b3pre
Status: RESOLVED → VERIFIED
Flags: in-litmus?(ayanshah62)
Depends on: 668201
You need to log in before you can comment on or make changes to this bug.