814442 - URLs cannot be clicked on in an email

Status: VERIFIED FIXED

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

Description

[Ian Barlow (:ibarlow)]

Steps to reproduce
1. Open an email to read
2. Read email, notice URLs in email

Expected results
1. URLs are visually distinct from the rest of email text
2. Tapping on the URL opens the page in Browser

Actual results
URLs cannot be interacted with at all.

Comment 1

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

What builds are you using?  This should work.

There are three separate code-paths that could be active depending on the type of the message:
1) plaintext message (text/plain without alternative)
2) HTML message, non-"newsletter" mode (text/html with or without alternative, layout does not indicate a desired width)
3) HTML message, newsletter mode (text/html with or without alternative, layout does indicate a desired width, causing pinch-zoom and funky panning to happen)

Please identify the type of message you are trying to display links in and whether the links in question are something we automatically linkified or something that was already linked in the e-mail.  You will want to use a desktop e-mail client and do view source/view original etc. to determine what's what.  Feel free to attach the e-mail if you're not sure, but do describe what links you tried to click on.

Comment 2

[David Scravaglieri [:scravag]]

Please could you check if it'ce reproducible ?

Comment 3

[Ian Barlow (:ibarlow)]

I would try, but I can't seem to get any email into my inbox now. I logged out and logged in sucessfully, but no mail is showing up :(

Comment 4

[Steve Chung [:steveck]]

(In reply to Andrew Sutherland (:asuth) from comment #1)
> What builds are you using?  This should work.
> 
> There are three separate code-paths that could be active depending on the
> type of the message:
> 1) plaintext message (text/plain without alternative)
> 2) HTML message, non-"newsletter" mode (text/html with or without
> alternative, layout does not indicate a desired width)
> 3) HTML message, newsletter mode (text/html with or without alternative,
> layout does indicate a desired width, causing pinch-zoom and funky panning
> to happen)
I think (3) HTML message with newsletter mode might have some problem when tapping on the URL because we have an interacter div above iframe body for gesture like pan/zoom/double click. Element inside the iframe body could not receive any click event and we don't perform any specific handling in this situation.

Comment 5

[Steve Chung [:steveck]]

(In reply to Ian Barlow (:ibarlow) from comment #3)
> I would try, but I can't seem to get any email into my inbox now. I logged
> out and logged in sucessfully, but no mail is showing up :(

I also face this problem in the latest build with my personal hotmail account (4000+ mail). It only works with my gmail account or testing hotmail account (20+ mail). Maybe you could try gmail first.

Comment 6

[Jim Porter (:squib)]

(In reply to Steve Chung from comment #5)
> I also face this problem in the latest build with my personal hotmail
> account (4000+ mail). It only works with my gmail account or testing hotmail
> account (20+ mail). Maybe you could try gmail first.

I'd be interested in seeing a logcat for your personal hotmail account if you can provide one.

Comment 7

[Steve Chung [:steveck]]

Attachment: hotmail loading message log

Here is the log that dumped while stuck on loading message.
The progress bar stop around 80%.

Comment 8

[Jim Porter (:squib)]

If you actually get to 80%, then all the messages were downloaded, and something imploded while processing the messages to add them to the database.

Comment 9

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

I'm not sure if the lines like:
[JavaScript Warning: "CSP WARN:  Directive style-src app://email.gaiamobile.org violated by http://addons.books.com.tw/G/auto_activity_new/act0000037/green_yellow.css"]

could have something to do with it, but they are concerning.  Since the sanitizer should be 1) destroying 'style' tags and 2) never passing through a 'src' attribute intact, this suggests the CSP is freaking out on our data document we create, which I would think should not be subject to that so much.

Comment 10

[Jim Porter (:squib)]

(In reply to Jim Porter (:squib) from comment #8)
> If you actually get to 80%, then all the messages were downloaded, and
> something imploded while processing the messages to add them to the database.

Steve: my guess is that there's a message that's causing us to throw an exception, but the exception isn't being logged. Probably the best way to debug this would be to open up apps/email/js/ext/gaia-email-opt.js, search for _enumerateFolderChanges, and then scroll down a bit and add some logging and/or a try block to the callback in the call to postCommand.

If you prefer, I can create a patch for you to apply, but I'm not entirely sure where the error would be occurring, so it might require some tinkering. If you find an issue, can you file a new bug and CC me on it?

Comment 11

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

(In reply to Jim Porter (:squib) from comment #10)
> If you prefer, I can create a patch for you to apply, but I'm not entirely
> sure where the error would be occurring, so it might require some tinkering.
> If you find an issue, can you file a new bug and CC me on it?

I think we probably want try/catch handlers wrapping _parseMessage anyways.  There's certainly no benefit to us exploding there.

We probably also either want to wrap e.run() in a try/catch or modify wbxml.js's run() so that it wraps its listener callback invocations in a try/catch.  Modifying run() may be the best option, and then if we have it support an onerror listener or something like that, that's where we could put the error logging in gelam.  (And wbxml.js could have a default to call console.error or re-throw the errors or something like that.)

Comment 12

[Steve Chung [:steveck]]

(In reply to Jim Porter (:squib) from comment #10)
> (In reply to Jim Porter (:squib) from comment #8)
> > If you actually get to 80%, then all the messages were downloaded, and
> > something imploded while processing the messages to add them to the database.
> 
> Steve: my guess is that there's a message that's causing us to throw an
> exception, but the exception isn't being logged. Probably the best way to
> debug this would be to open up apps/email/js/ext/gaia-email-opt.js, search
> for _enumerateFolderChanges, and then scroll down a bit and add some logging
> and/or a try block to the callback in the call to postCommand.
> 
> If you prefer, I can create a patch for you to apply, but I'm not entirely
> sure where the error would be occurring, so it might require some tinkering.
> If you find an issue, can you file a new bug and CC me on it?

Since not all the account would be failed to receive messages, I will try to apply try/catch in the block you said and create a bug if the exception thrown out.

Comment 13

[Steve Chung [:steveck]]

Attachment: Patch for making URL link clickable in email

Comment 14

[Steve Chung [:steveck]]

(In reply to Andrew Sutherland (:asuth) from comment #11)
> I think we probably want try/catch handlers wrapping _parseMessage anyways. 
> There's certainly no benefit to us exploding there.
> 
> We probably also either want to wrap e.run() in a try/catch or modify
> wbxml.js's run() so that it wraps its listener callback invocations in a
> try/catch.  Modifying run() may be the best option, and then if we have it
> support an onerror listener or something like that, that's where we could
> put the error logging in gelam.  (And wbxml.js could have a default to call
> console.error or re-throw the errors or something like that.)

It did stuck in _parseMessage with JavaScript CSP Warning. But I've tried to use try/catch in parseMessage but I could not catch any exception.

Comment 15

[mlevin]

Verified fixed on Unagi Build 20121231070201