585875 - Extend Fennec's copy/paste support

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Antti Järvelin]

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

Comment 1

[Antti Järvelin]

Opening a new bug for Fennec because Bug 582912 is set RESOLVED FIXED, but the patch for Fennec is not yet processed.

Comment 2

[Antti Järvelin]

Attachment: Fix Proposal for fennec

This patch extends the copy/paste support of fennec.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Not a dupe. See comment 1.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

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.

Comment 6

[Jeremias Bosch (:jbos)]

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.

Comment 7

[Martijn Wargers (dead)]

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.

Comment 8

[Matt Brubeck (:mbrubeck)]

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

Comment 9

[Antti Järvelin]

What are the plans with this patch? Is it going to be integrated any time soon?

Comment 10

[Madhava Enros [:madhava]]

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.

Comment 11

[Madhava Enros [:madhava]]

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.

Comment 13

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

http://wiki.maemo.org/N900_shortcuts_and_gestures

Comment 14

[Stefan Wrobel]

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.

Comment 16

[Matt Brubeck (:mbrubeck)]

Attachment: Context menu for chrome textboxes (WIP)

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?

Comment 17

[Mark Finkle (:mfinkle) (use needinfo?)]

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.

Comment 18

[Stefan Wrobel]

I can't believe Fennec is at version 4.0pre on Android and still doesn't have copy/paste!!!

Comment 19

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 464340 [details] [diff] [review]
Fix Proposal for fennec

Let's take a smaller version.

Comment 20

[Mark Finkle (:mfinkle) (use needinfo?)]

Matt - just checking on the status

Comment 21

[Matt Brubeck (:mbrubeck)]

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.

Comment 22

[Brian Crowder]

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)

Comment 23

[Matt Brubeck (:mbrubeck)]

Attachment: Context menu for chrome textboxes

Fully working version of the previous patch.  This is still for chrome only.

Comment 24

[Mark Finkle (:mfinkle) (use needinfo?)]

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.

Comment 25

[Matt Brubeck (:mbrubeck)]

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.

Comment 26

[Matt Brubeck (:mbrubeck)]

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.

Comment 27

[Matt Brubeck (:mbrubeck)]

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

Comment 28

[Matt Brubeck (:mbrubeck)]

Attachment: part 2: Fix long-tap behavior for Android

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.

Comment 29

[Mark Finkle (:mfinkle) (use needinfo?)]

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.

Comment 30

[Mark Finkle (:mfinkle) (use needinfo?)]

Forgot to ask. Do we still need "ui.click_hold_context_menus = true" ?

Comment 31

[Matt Brubeck (:mbrubeck)]

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

Comment 32

[Matt Brubeck (:mbrubeck)]

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

Comment 33

[Aakash Desai [:aakashd]]

\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