985093 - [email] URI parsing in new/share compose activity using URI does not catch encoding errors. may need to switch to friendly decoder to avoid all-or-nothing failures

Status: VERIFIED FIXED

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

Description

[Chris Kreinbring [:CKreinbring]]

Attachment: Log of launching email from Weather Channel

Description:
Attempting to launch the Email app from the Weather Channel app while signed in with an email account will show a blank black screen instead of the user's inbox.

Repro Steps:
1) Update Buri to Build ID: 20140317040204
2) Launch the Email app and sign in with a valid username and password.
3) Return to the homescreen, launch the Marketplace, and download the Weather Channel app.
4) Return to the homescreen and launch the Weather Channel bug.
5) Navigate through the setup until the city weather page is reached.
6) Scroll to the bottom of the page and tap the email icon.
7) Observe the appearance of the email app as it is launched.

Actual:
The email app launches and shows a blank black screen.

Expected:
The email app launches and shows the user's inbox.

Environmental Variables
Device: Buri 1.4 mozilla RIL
Build ID: 20140317040204
Gecko: https://hg.mozilla.org/mozilla-central/rev/25cfa01ba054
Gaia: 8f802237927c7d5e024fb7dca054dd5efef6b2e6
Platform Version: 30.0a1
Firmware Version: V1.2-device.cfg

Notes:
Repro frequency: 100%
See attached video clips and logcat logs

Comment 1

[Chris Kreinbring [:CKreinbring]]

Does not repro on Buri 1.3 mozilla RIL.  The inbox appears with no errors.

Build ID: 20140318004002
Gecko: https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/cf2042938526
Gaia: 2ea2aab306bd1c941719160cdcb49ee9d755dc17
Platform Version: 28.0
Firmware Version: V1.2-device.cfg

Comment 2

[Chris Kreinbring [:CKreinbring]]

Attachment: Video of launching email from Weather Channel (not logged in)

Comment 3

[Chris Kreinbring [:CKreinbring]]

Attachment: Video of launching email from Weather Channel (logged in)

Comment 4

[Jason Smith [:jsmith]]

comment 1 says it doesn't repro on 1.3, but the status flags says it does repro on 1.3. Which is it?

Comment 5

[Chris Kreinbring [:CKreinbring]]

The wrong flag was set.  Fixed now.

Comment 6

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

We don't like the URI we are seeing and are exploding, which is our bug.

The URI we get is:
mailto:?subject=Share%20Weather&body=Checkout%20the%20latest%20weather%20updates%20at%20http%3A%2F%m.weather.co.uk

The problem is that "%m" is not a legit encoding.

The log excerpt which makes this clear:
03-18 12:31:52.549: I/GeckoDump(575): [32mLOG: Received activity: new[0m
03-18 12:31:52.849: I/GeckoDump(575): [31mERR: onerror reporting: URIError: malformed URI sequence @ app://email.gaiamobile.org/js/mail_app.js : 557[0m
03-18 12:31:52.849: E/GeckoConsole(575): [JavaScript Error: "URIError: malformed URI sequence" {file: "app://email.gaiamobile.org/js/mail_app.js" line: 557}]

Comment 7

[Peter Bylenga [:PBylenga]]

This issue has been occurring since the first v1.4 that we have available though it's a white screen instead of a black screen.
 
v1.4 Environmental Variables:
Device: Buri v1.4 MOZ
BuildID: 20131217040201
Gaia: 545aacf3feff6430140cc9ade757002df4895b77
Gecko: b1e5ade62913
Version: 29.0a1
Firmware Version: v1.2-device.cfg

Comment 8

[James Burke [:jrburke]]

Attachment: GitHub pull request

This fix protects us against bad mailto: URL input from sites.

Our partner app team should follow up with the Weather Channel people though and have them fix their mailto: link. While this pull request fix prevents a black screen in the mail app, it means the email body is empty.

Even if we had a fancier decoder (although we use decodeURIComponent in the platform, which is the standard implementation for decoding), we would still not show a workable URL for their message, so their ultimate intent of sharing a usable Weather Channel URL with people would not be met.

So the most complete fix is for the Weather Channel to fix their mailto link. This change just makes sure the email app does not look broken or crashed.

This bug is also unrelated to the changes in bug 969609.

Comment 9

[Jason Smith [:jsmith]]

Clearing regression window since we've already got a patch here.

Comment 10

[James Burke [:jrburke]]

Merged to gaia master:
https://github.com/mozilla-b2g/gaia/commit/49caf2a61e0e7c4de5d919c841b985576a8e53c4

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

Comment 11

[Jason Smith [:jsmith]]

This is needed to be uplifted in order to safely fix bug 969609 without this blocking regression, so moving to 1.3+. We'll need approval on this.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

v1.4: https://github.com/mozilla-b2g/gaia/commit/cb731747d3e6cd91d3b244ad02ab8d3a11eb5fe2

Comment 13

[Ryan VanderMeulen [:RyanVM]]

(In reply to Jason Smith [:jsmith] from comment #11)
> We'll need approval on this.

Please request approval-gaia-v1.3 on this :)

Comment 14

[James Burke [:jrburke]]

Comment on attachment 8393249 [details] [review]
GitHub pull request

This bug uplift will enable bug 969609, also a 1.3+ bug, to land and work without undesirable side effects.
 
[Bug caused by] (feature/regressing bug #):
A non-gaia app that had an incorrectly URI-encoded mailto: URL. When bug 969609 is uplifted to 1.3+, that failure results in a blank screen. But even without bug 969609 uplifted, the mailto: jump would still fail and the user would not get to the Compose screen.

[User impact] if declined:
The weather app in question will continue to fail to get to a Compose screen, and if bug 969609 is uplifted, result in a blank UI.

[Testing completed]:
Tested on device during dev.

[Risk to taking this patch] (and alternatives if risky):
Low risk. As it is about wrapping decodeURIComponent in a try/catch, safer code than before, and isolated to mailto: parsing.

[String changes made]:
none

Comment 15

[Roland Kunkel [:RolandK]]

Verified as fixed, v1.5. I am unable to reproduce this issue in the latest v1.5 Master Buri build:

4/2 Environmental Variables:
Device: Buri 1.5 MOZ RIL
BuildID: 20140402040201
Gaia: 6274b3645b3d41df4aa5a55398b5d106edf1c528
Gecko: 4941a2ac0786
Version: 31.0a1
Firmware Version: V1.2-device.cfg

-

Verified as fixed, v1.4. I am unable to reproduce this issue on the latest v1.4 Buri build:

4/2 Environmental Variables:
Device: Buri 1.4 MOZ RIL
BuildID: 20140402000202
Gaia: 1db7a4857faec427799de0827964128b445e80ea
Gecko: b1b159c885cc
Version: 30.0a2
Firmware Version: V1.2-device.cfg

-

1) Verified against Master v1.5, changing status to verified.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

v1.3: https://github.com/mozilla-b2g/gaia/commit/b1e284e860c6364a3e47f627012a65c977d642ee