Closed Bug 1092440 Opened 5 years ago Closed 5 years ago

Email should follow text selection pattern.

Categories

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

x86
macOS
defect
Not set

Tracking

(feature-b2g:2.2+, ux-b2g:2.2)

VERIFIED FIXED
2.2 S1 (5dec)
feature-b2g 2.2+
ux-b2g 2.2

People

(Reporter: swilkes, Assigned: jrburke)

References

Details

Attachments

(3 files)

See attached spec.

This may need to be deferred until the Clipboard API is available (which will be 2.2 or later).

User should be able to select all email text, but user should be able to select message text separately (see spec) and not be forced to select all messages that reside in the selectable area. 

* Input fields (input areas, input dialogs and search fields) are the only Building Blocks with selectable text. 

* HTML elements like buttons or links, or events like ontouchstart or onclick, should trigger the action when tapped rather than triggering text selection.
HTML mails are going to make this pretty complex.

There's basically two roads for selection, I suspect:

1) Try and rely on the system; like if the web browser has good selection mechanisms, we're theoretically home free.  The complicating factors are that right now our iframe has a scale() transform applied to it and is "pointer-events: none" with us manually scaling the click coordinate space and figuring out what the element is so that we can then prompt the user if they're sure they want to browse there.  If the browser app takes on the anti-phishing security/privacy aspects, we could potentially drop some of this.  This will also likely require the ability to stop doing the transform scale() hack and instead have a proper full accelerated APZ nested mozbrowser.  (Note that there are some complexities related to defeating external resource loads that might complicate various parts of this.)

2) Use a custom selection UI.  Like on long-press or whatever we bring up our affordance:
a) Easier: Just convert the whole thing to a big unformatted textarea and let the user use the native affordances on that.
b) Harder: Let the user draw a selection rectangle over the text and we then use that as a basis for manually using the DOM selection APIs on the basis of what intersects our big bounding box.  We could transform the coordinate space and everything for the existing transform scale hack.  There might also be some potential rough edges we could handle here.
c) Other: Assuming we have an HTML editing widget available to us that has its own copy/paste mehanics (and is safe!), kick the user into the editor and leave it up to the editor.  In theory this is less horrible than 2a because it should still generally resemble HTML.  This may be the hardest of them all, but could dovetail with supporting HTML editing (which is potentially difficult if done generally by the system, but there are potentially third party libs we could use.)
Blocks: gaia-copy-paste
No longer blocks: CopyPasteLegacy
feature-b2g: --- → 2.2?
The Gecko work is close to completion, the remaining part is to fix test case failures then pref it on in m-c bug 1092888. After the bug landed, text selection and cut/copy/paste can be triggered in apps on both editable and non-editable elements.

Gaia per app work in v2.2 is to follow UX guideline and leverage CSS -moz-user-select https://developer.mozilla.org/en-US/docs/Web/CSS/user-select to make certain elements non-selectable.

Before Gecko is pref on in m-c, Gaia can manually switch it on by the following two pref to test in advance:
pref("selectioncaret.enabled", on);
pref("selectioncaret.noneditable", on);
This is a 2.2 feature.
feature-b2g: 2.2? → 2.2+
Attached file GitHub pull request
This pull request sets it up so that no elements are user selectable as a baseline, then selectively enables:

* The input text entries we have. I included "url" just in case we have one later.
* I enabled text selection in the text entries for the signature, and for the email body compose area. I did this to allow easy cut/paste capabilities while composing text in those fields.
* Plain text email bodies in the message_reader are selectable. I chose to use a * selector to allow that structure to change. I originally had a -moz-user-select style on each of the selectors in the CSS for those sub-DOM elements, but it seemed fragile to change later. So I am trading future flexibility for a slower selector. I am fine taking the tradeoff the other way though. Do not see a perf hit at user scale.

I originally tried just selecting `-moz-user-select: none` on the body, hoping other elements would inherit it but that did not seem to work. So I went with the * selectors.

I tested copying rich text HTML from a web page and pasting it in our compose or signature card areas, and luckily the paste is text only, no HTML tags. So woohoo for that, no changes needed there.

I had a change to add  "copy" menu item to the peep bubble contact menu, and dispatched a ClipboardEvent (https://developer.mozilla.org/en-US/docs/Web/API/ClipboardEvent.ClipboardEvent) on `document` for a 'copy' with the peep.address, but that did not seem to lead to it showing up as an option for paste text. So I did not include that change here, may need some upgrades to the copy/paste plumbing. That or more education on the incantations I need to do for 

The other outstanding issue are HTML email bodies: those are not selectable. While it is somewhat nice to have plain text bodies selectable, in practice there are emails that may look like they are just plain text, but fall into to the iframe-based HTML email body display, so not sure how useful it is in total.

I looked more into the iframe display, and it is as :asuth already mentioned in comment 2: we are really stuck by the zoom code and the tap overrides we do for links. I had thought about maybe moving the tap/zoom code into the iframe, but that really goes against keeping that iframe clear of script, likely has script origin reference issues too.

--- Summary ---

We should file a follow on bug to track improving the HTML body text selection situation, but we likely need platform help to go to a real mozbrowser for a full fix. I would say it is not likely to see that land in 2.2. 

With this pull request though, we can get everything else set up. If I can get some guidance on if dispatching ClipboardEvents or some other API for the name bubbles in the message reader are the way to get that done, then I can get that in pretty quickly. ni? :bhuang for guidance if there is an API for that in the 2.2 timeframe. I thought I saw another bug that said that the clipboard API will not be available until 2.3.

Not asking for dev review yet until I sort out the clipboard stuff.
Flags: needinfo?(bhuang)
feature-b2g: 2.2+ → 2.2?
I filed bug 1102469 to track an ultimate fix for HTML email viewing.
feature-b2g: 2.2? → 2.2+
Target Milestone: --- → 2.2 S1 (5dec)
It's unlikely we'll have an API ready for 2.2, and after some tests it doesn't seem like there is a way to programmatically select a range.  
One question, if a user forwards or replies to an HTML email, is that content then selectable in the editable field?
Flags: needinfo?(bhuang) → needinfo?(jrburke)
(In reply to Bruce Huang [:bhuang] <bhuang@mozilla.com> from comment #6)  
> One question, if a user forwards or replies to an HTML email, is that
> content then selectable in the editable field?

If it is a plain text email, the block commented text is selectable and editable. If it is an HTML email though, we need to use the iframe to show it, and therefore follows under the same issues as the original display of the HTML email. So it is uneditable.

So given that the API will not be in 2.2, and we have bug 1102469 to track the HTML email, I think the pull request for this bug is ready for review, so I will set that up next.

Bruce, if you have a bug that tracks the copy/paste API that could be used for the text bubbles, mention it here, and I can create an email bug to fix the bubbles, and mark it as depending on that bug.
Flags: needinfo?(jrburke) → needinfo?(bhuang)
Comment on attachment 8524305 [details] [review]
GitHub pull request

Flipping on review, notes in the pull request.

For people that do not want to build the pull request, you can try out the state of it by installing this app zip via the WebIDE tool (this assumes you already enabled the prefs to enable the text selection tools):

http://jrburke.com/work/gaia/email-text-sel.zip

This review should be considered a low priority review for the reviewer, I know we have some bigger backend code changes in the work. If you prefer that I turn off review until next sprint, to cut down on mental load, just let me know.
Attachment #8524305 - Flags: review?(bugmail)
Comment on attachment 8524305 [details] [review]
GitHub pull request

re: rich content, yes, we are definitely lucky there.  I also tested that to be doubly sure.

The changes seems sane and this generally seems to work, so let's ship it! :)

Note that on a recent trunk gecko/gaia it does seem impossibly hard to trigger a paste in an empty input box, but that seems to be a system-wide issue.  Likewise, the cut/copy/paste/affordance seems to want to stick around a lot longer than it should.  (The control was destroyed so much have been blurred, right?)

I think those are out of our hands for now, but at least in the compose UI, I do worry that all our cleverness about automatically focusing things on clicks could cause trouble for standard system heuristics, depending on what the actual way to trigger copy/paste is.  (I just long press and hope.)
Attachment #8524305 - Flags: review?(bugmail) → review+
Merge in master:
https://github.com/mozilla-b2g/gaia/commit/8cbb8cc6586e6b172eff8df604458da412d7c906

from pull request:
https://github.com/mozilla-b2g/gaia/pull/26213

with the caveats that bug 1102469 needs to be solved for HTML email text selection, and the copy/paste API needs to be available for the contacts copy to work, both likely not to be solved in 2.2 I will file the copy/paste API usage bug once I know what bug :bhuang prefers to use as the blocker bug for the API.
Assignee: nobody → jrburke
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
QA Whiteboard: [2.2-feature-qa+] [textselection]
Attached image screenshots.zip
Gaia-Rev        41b7be7c67167f367c3c4982ff08651d55455373
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/ff4a63d66806
Build-ID        20141126160201
Version         36.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  39
FW-Date         Thu Oct 16 18:19:14 CST 2014
Bootloader      L1TC00011880

See attached file for screen shots of text selection in Email.
1 - shows when text is selected.
2 - shows when text is deselected by clicking anywhere in the screen.
3 - shows when text is selected and copy is clicked.
4 - shows when text is selected and select all is clicked.
5 - shows when sender email is double clicked. 


There are some concerns:
2 - Icons remains after text is deselected, and stays on screen.
3 - Blue markers remain after copy is clicked.
4 - There is only one blue marker on screen and in a awkward position.
5 - Need to verify if this is the right behavior for sender and recipient fields. That is, text selection is not enabled.   

Thanks.
Flags: needinfo?(jshen)
Flags: needinfo?(jrburke)
The text selection not enabled for to/recipients in the message_reader is the "need copy/paste API for contacts" bug that needs to be filed, mentioned in comment 10. 

I was holding off on filing until I got the copy/paste API bug, but I went ahead and filed the issue as bug 1105852, and we can mark it as depending on the copy/paste API once we know it. That will not be in 2.2 though.

The other issues around actual selection and the appearance or disappearance of the copy/paste buttons are more about the platform behavior, so :bhuang might provide better answers for those. The email app does not have control over those behaviors though.
Flags: needinfo?(jrburke)
Flags: needinfo?(jshen)
Flags: needinfo?(bhuang)
QA Whiteboard: [2.2-feature-qa+] [textselection] → [2.2-feature-qa+] [COM=Text Selection]
Resolve verified as per comment 11.
Opened bug 1146262 for tracking as per 4. in comment 11.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.