872500 - E-mail application offers to open all links (use 'pattern' instead of 'regexp in manifest)

Status: RESOLVED FIXED

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

Description

[Chris Lord [:cwiiis]]

When clicking a link in an application, such as Twitter, a dialog pops up offering to open it in the browser or the e-mail application. This happens for all links. master is affected, last time I checked, this was fine on v1-train.

Comment 1

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

Bug 853019 landed in mozilla-central on March 27th which means this affects Firefox/Gecko 22 and later.

Comment 2

[Andrea Marchesini [:baku]]

Andrew, how can this be related to this problem? If that code need to be updated, I can take care of it...

Comment 3

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

https://github.com/mozilla-b2g/gaia/blob/master/apps/email/manifest.webapp#L61

We need to add "pattern" to the url thing since it is used instead of "regexp" on firefox 22+.  We may need to change the syntax too.  (It sounds like pattern may already include the '^' so we might not need that.  MDN's docs were not immediately clear and other apps like the browser have not been updated yet.)

Comment 4

[Andrea Marchesini [:baku]]

> We need to add "pattern" to the url thing since it is used instead of
> "regexp" on firefox 22+.  We may need to change the syntax too.  (It sounds
> like pattern may already include the '^' so we might not need that.  MDN's
> docs were not immediately clear and other apps like the browser have not
> been updated yet.)

Right. The pattern string is converted in ^(?:' + pattern + ')$'

We need to update:

browser/manifest.webapp
        "url": { "required":true, "regexp":"/^https?:.{1,16384}$/" }
        "url": { "required":true, "pattern":"https?:.{1,16384}" }

communications/manifest.webapp
        "number": { "regexp":"/^[\\d\\s+#*().-]{0,50}$/" }
        "number": { "pattern":"[\\d\\s+#*().-]{0,50}" }

email/manifest.webapp
        "url": { "required":true, "regexp":"/^mailto:/" }
        "url": { "required":true, "pattern":"mailto:.{1,16384}" }

homescreen/manifest.webapp
        "url": { "required":true, "regexp":"/^https?:/" }
        "url": { "required":true, "pattern":"https?:.{1,16384}" }

sms/manifest.webapp
        "number": { "regexp":"/^[\\w\\s+#*().-]{0,50}$/" }
        "number": { "pattern":"[\\w\\s+#*().-]{0,50}" }

We need a double check for these regexps.

tef? leo?

Comment 5

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

The change is only on firefox 22+; so we want to add pattern and not remove regexp until firefox 18 is long gone, I believe.  If you're going to do all apps, I think you'll want to do that on bug 857059.

Because b2g18 is fine, neither tef or leo needs the fix.

Comment 6

[Julien Wajsberg [:julienw] (PTO -> Mar 16)]

Sms case is handled in the patch for Bug 806490.

Comment 9

[Kevin Grandon :kgrandon]

Attachment: Github pull request pointer

This bug was driving me crazy. Baku would you be comfortable reviewing this one?

Comment 10

[Andrea Marchesini [:baku]]

Comment on attachment 782831 [details]
Github pull request pointer

lgtm

Comment 11

[Kevin Grandon :kgrandon]

Landed in master: https://github.com/mozilla-b2g/gaia/commit/6ed86aef1ecfe15178bb1cfb358db1fa45febe7e

Comment 14

[Julien Wajsberg [:julienw] (PTO -> Mar 16)]

So Kevin you just broke the compatibility with b2g18, right ? :)

Could it be possible to keep both properties "regexp" and "pattern" (as was the case in the Sms app btw) ?

Comment 16

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

(In reply to Julien Wajsberg [:julienw] from comment #14)
> So Kevin you just broke the compatibility with b2g18, right ? :)
> 
> Could it be possible to keep both properties "regexp" and "pattern" (as was
> the case in the Sms app btw) ?

Ignoring the dev-gaia thread about gaia/master support on b2g18,  I think the right call to make here is based on what provides the best example for people out there developing apps for Firefox OS who look to the Gaia apps for guidance.  Since we are shipping v1.0.1 devices and v1.1 devices and those need the old way still, I think we should keep both around.

Comment 17

[Kevin Grandon :kgrandon]

Someone can back my patch out if you wish, but I don't think we should leave dead-code in master simply for "guidance".

In my opinion this should be solved with documentation and communication to developers.

Comment 18

[Julien Wajsberg [:julienw] (PTO -> Mar 16)]

I just changed https://developer.mozilla.org/en-US/docs/WebAPI/Web_Activities#Activity_handler_description which didn't mention "regexp" at all. I think this is way to buried to be useful and that we should keep both in our manifest. Plus we should add a patternFlag for the browser activity.

Should we file a new bug for this ?

Comment 19

[[:fabrice] Fabrice Desré]

Keeping both in the manifests just add to the confusion. gaia-master is 1.2, there's no reason to keep old cruft there.