891641 - [B2G] [Leo] [Email] Browser links in received emails are unselectable if file attachments are present

Status: VERIFIED FIXED

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

Description

[Chris Kreinbring [:CKreinbring]]

Attachment: Log of receiving email and selecting browser link

Description:
The user is unable to select browser links in received emails if the emails also have file attachments added to them.

Repro Steps:
1) Updated to Leo Build ID: 20130709070206
2) Launch the Email app and log in to a valid account if needed.
3) Send an email to the account containing an attachment and a link.
4) Tap Refresh on the device and open the email that appears.
5) Attempt to open the link.

Actual:
Nothing happens.

Expected:
A warning page appears confirming if the user wants to go to the link that was just selected.

Environmental Variables
Occurs on Leo 1.1 commercial RIL
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/f917f3fb17ab
Gaia: e251ee6bdab13d8620afa8f9c2d5f14e5e6a4f99
Platform Version: 18.1
RIL Version: 01.01.00.019.152 

Notes:
Repro frequency: 100%
Q Analysts Team Priority: Pri 2
See attached logcat logs

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

confirmed on gaia/v1-train

There are 2 weird/bad problems going on here:

- The attachment download button is acting all position:fixed regardless of body type; when scrolling the display, the button stays in its screen position.  It also seems to move around as a result of our iframe transforms.  It looks like our use of position: absolute while only specifying 'right' as a way to position the icon back-fires, since this takes the icon out of the normal flow processing for scrolling.  Oops!

- The coordinate space transform we are using for text/html body parts appears to be using the wrong frame of reference for all click procesing (regardless of whether we created the 'a' link or not.)  Specifically, if I make a message with 1 link per text line, clicking on the first line nets me an apparent click on the 4th or 5th lines.  The problem is some combination of the .msg-attachments-container "ul" being outside of .msg-envelope-bar (which we do account for), or our only using specific DOM node heights rather than trying to determine the absolute position of our iframe using offsetTop/offsetLeft-type values.

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

note that the weird download button behaviour is only happening on v1-train for me, not gaia/master (although the download button did seem to lag behind by a paint cycle in at least one case?).  There might be something we can/should uplift to stop that weirdness from happening.

note that you'll need a gaia/master from an hour or two in the future from now, because jrburke has an AMDification bug fix that needs to land before you can see attachments on master right now.

Comment 3

[Wayne Chang [:wchang]]

Triage - Leo+ as functional failure

Comment 4

[psingapati]

Attachment: Links with Attachment

Issue is reproduced in the Gaia/Master also.

The issue observed is, 
1. Link is not selected so easily, when there is an attachment
2 [review]. If there are multiple links, when we select on a first link , second link is shown on the dialog.

Gaia Revision: b3ef9da9c967e8f33b7b750d986d395e1aa38c95

Attaching screen shot for the attachments and links position.

Please check and let me know, if you need any more information.

Comment 5

[psingapati]

Please ignore the [details] [diff][review] part in the second point, 
Something went wrong.

Comment 6

[sbogati]

Issue no longer occurring on
Leo v1.1.0 COM RIL
Unagi Build ID: 20130711070209
Kernel Date: Jun 20
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/aaa74e5efaf1
Gaia: 7cdcc46179d198cab11970964b181ede32a5b683
Platform Version: 18.1
RIL Version: 01.01.00.019.158

Comment 7

[sbogati]

resolving as WFM per comment 6

Comment 8

[Nick Davidson]

Issue still repro on Build ID: 20130712070210
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/282b5c37cf8d
Gaia: e2ef782119b7e79fc62c48d36f0c36909d982988
Platform Version: 18.1
RIL Version: 01.01.00.019.158

Issue happens on email accounts regardless if email has attachments or not. 1)If you tap on a link in email nothing happens.
OR -
2)User will get confirmation screen asking Brows to URL?: (...google.com)
user pressing OK but still stays in email app in email view window even he can see data transmission arrows moving on a top of a screen. Email app does not switching user to browser app. If user closing email app and tapping on a browser icon from home screen user can see browser page with a link from email  opened in browser.

Comment 9

[Alex Keybl [:akeybl]]

Nick, please provide full steps to reproduce.

Comment 10

[Nick Davidson]

Detailed repro steps:

1) Set 2 email accounts on a device - 1 @Gmail.com (IMAP) another one @outlook.com (ActiveSync)
2) Send Email with random web links in email body to those two email addresses ( for example: http://bensbargains.net

http://seattle.craigslist.org/

http://www.dhgate.com/product/car-2-x-8-led-drl-car-daytime-running-lights/152956620.html

https://bugzilla.mozilla.org/show_bug.cgi?id=891641

http://www.dhgate.com/product/car-2-x-8-led-drl-car-daytime-running-lights/152956620.html

http://www.techhive.com/article/2044127/tmobile-to-launch-its-first-firefox-phone-next-week.html

http://youtu.be/E_nbLg2U0Ho )

3) Open email with this links in each email account and try to tap on a links in email body to open those links in a browser.
4) Observe what happens

There is 2 possible outcomes:

1)If you tap on a link in email nothing happens.
OR -
2)User will get confirmation screen asking Brows to URL?: (...google.com)
user pressing OK but still stays in email app in email view window even he can see data transmission arrows moving on a top of a screen. Email app is not switching user to browser app. If user closing email app and tapping on a browser icon from home screen user can see browser page with a link from email  opened in browser.
See YouTube video: http://youtu.be/OVTEbMqvvlY

Environmental Variables
Build ID: 20130715070218
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/6062fdf2deb8
Gaia: 55ed5e08a2250ea2d3571fff860c39e66fabed14
Platform Version: 18.1
RIL Version: 01.01.00.019.158

Comment 11

[Nick Davidson]

Also issue with no opening links in email happens more often with ActiveSynk email 

it is less frequent in IMAP (about 30-40%)- 
I would say it is works correctly on Gmail about 20% when expected behavior is when you tap on a link and confirm that you want yo open this link, browser app is opened with corresponding link.
The rest of the times (40-50%) after you confirm you want to open link it does get opened in a browser but task is not switching from email to browser app automatically.

Comment 12

[Andrew Sutherland [:asuth] (he/him)]

This is a deterministic failure on text/html attachment parts per my investigation in comment 1.  Failure to open the link would be a different bug, possibly in the system app.

Comment 13

[[e-mail], [e-mail]]

(In reply to Andrew Sutherland (:asuth) from comment #12)
> investigation in comment 1.  Failure to open the link would be a different
> bug, possibly in the system app.
Yes, I can reproduce it in Settings app -> Help -> User guide. The symptom is able to be reproduced while Browser app already launch in background. If Browser app does not launch, Email app switching user to Browser app normally. Let's separate the section to another issue.

Bug 894783 - [Browser] Cannot switching user to browser app while having an activity "view" request and Browser app in background

Comment 14

[[e-mail], [e-mail]]

(In reply to Andrew Sutherland (:asuth) from comment #1)
> confirmed on gaia/v1-train
> 
> - The coordinate space transform we are using for text/html body parts
> appears to be using the wrong frame of reference for all click procesing
> (regardless of whether we created the 'a' link or not.)  Specifically, if I
> make a message with 1 link per text line, clicking on the first line nets me
> an apparent click on the 4th or 5th lines.  The problem is some combination
> of the .msg-attachments-container "ul" being outside of .msg-envelope-bar
> (which we do account for), or our only using specific DOM node heights
> rather than trying to determine the absolute position of our iframe using
> offsetTop/offsetLeft-type values.
I think we should consider the height of .msg-attachments-container while identify clicking node for which <a>.(https://github.com/mozilla-b2g/gaia/blob/master/apps/email/js/iframe_shims.js#L394-L395)
The height is mistaken for 4~5 lines or the other margin/transform offset.
I was working on the above solution for considering .msg-attachments-container. Although I subtracted it, there is still not accurate enough for identifying clicking node. We can try to click the url without attachment case. The deviation is almost 1 line. I find out we don't consider the margin-top of .msg-body-container. It needs to be subtracted while computing dy. 

Beside of the two reasons, the offset(transform-origin: left top 0px) of transform iframe is another problem. It also increases deviation dy. But I have no idea for getting the transform offset. I will push my patch to Github soon.

Comment 15

[[e-mail], [e-mail]]

Attachment: WIP

Comment 16

[[e-mail], [e-mail]]

Comment on attachment 777821 [details]
WIP

Remove the feedback request, since find out the solution.

Comment 17

[[e-mail], [e-mail]]

Attachment: Pointer to Github pull request 11052.html

Hi Andrew,

Could you please help to review my pr? Thank you.

Comment 18

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 779120 [details]
Pointer to Github pull request 11052.html

This works well in my testing on Firefox nightly and a Leo device running mozilla-b2g18 using the patch from gaia/master.  I think it would be cleaner to try and do the math based off of the iframe or its parent rather than trying to subtract off the extra intermediary DOM, but I don't think it's worth doing another pass for this.  r=asuth for landing the patch.

An important note is that when I cherry-picked the patch to run against v1-train (so a Leo device with the most recent mozilla-b2g18 build and gaia/v1-train), the attachment download icon is still exhibiting the problems I described in comment 1 when scrolling.  Ian, if you could confirm that you see the problem when your patch is cherry-picked to v1-train and you scroll the message reader (so have a message at least a few lines long), I think we definitely need a spin-off bug for fixing that v1-train problem, so if you could file the bug then and nom it as a leo? blocker too...

Comment 19

[[e-mail], [e-mail]]

(In reply to Andrew Sutherland (:asuth) from comment #1)
> confirmed on gaia/v1-train
> 
> There are 2 weird/bad problems going on here:
> 
> - The attachment download button is acting all position:fixed regardless of
> body type; when scrolling the display, the button stays in its screen
I cannot reproduce the issue on v1-train. The button works fine when scrolling the message body. It looks like to be position: absolute. The header and attachments container are grouping together while scrolling.
> position.  It also seems to move around as a result of our iframe
> transforms.  It looks like our use of position: absolute while only
> specifying 'right' as a way to position the icon back-fires, since this
> takes the icon out of the normal flow processing for scrolling.  Oops!
> 
For the section, I'm able to understand the exactly symptom. Maybe a screenshot is helpful to me. I have tried to reproduce the specific symptom on v1-train after cheer-pick the patch. But it works fine for me.

Comment 20

[[e-mail], [e-mail]]

Thanks for Andrew's reviewing effort. Since pr is merged, we can close issue now.

master:   afde38e7be5455d5d634c58a60a4d8009e0e7034
v1-train: ffe25bfdf0e71c820ca710cb61fb8306564a8f4e

Comment 21

[Chris Kreinbring [:CKreinbring]]

Fix verified on Leo 1.1 commercial RIL.

Build ID: 20130723070209
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/68fb0a2e0114
Gaia: ffe25bfdf0e71c820ca710cb61fb8306564a8f4e
Platform Version: 18.1
RIL Version: 01.01.00.019.171

Comment 22

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Ian Liu [:ianliu] from comment #19)
> For the section, I'm able to understand the exactly symptom. Maybe a
> screenshot is helpful to me. I have tried to reproduce the specific symptom
> on v1-train after cheer-pick the patch. But it works fine for me.

It seems like there must be something weird with my build state.  I had :doliver try and reproduce and he didn't see anything like I was describing.  I'll try and figure it out just so I can stop seeing it on v1-train, but I don't think we need to do anything until testers start seeing it.

Comment 23

[John Ford [:jhford] CET/CEST Berlin Time]

v1.1.0hd: ffe25bfdf0e71c820ca710cb61fb8306564a8f4e