888260 - contacts pick activity displays empty dialog box when triggered from <input type="file"> or other requests to pick without an associated type

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::Contacts, defect)

Description

[Ben Kelly [:bkelly, not reviewing]]

As discussed in bug 883341, there are certain conditions that will cause the contacts pick activity to display an empty dialog box.

The contacts code expects that the activity will only be triggered if the type matches one of its filter values.  Currently, however, all apps providing the pick activity will be triggered when no type is specified.  In that case, contacts does not properly initialize its dialog message.

Contacts should have a decent, catch-all fallback message like "No data found".

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Nominating since this bug was broken out from bug 883341 which was leo+.

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

UX, do you have a preference on the default message?  Given timeframes for leo and l10n strings we are going to proceed with a basic message.

Comment 3

[:gerard-majax]

Attachment: Link to Github https://github.com/mozilla-b2g/gaia/pull/10700

Please find a link to Github https://github.com/mozilla-b2g/gaia/pull/10700

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

Two new strings that need to be localized in this bug:

  filetypeUnsupportedTitle   = Invalid type of data
  filetypeUnsupportedMessage = This application cannot handle the type of data requested.

Not sure of the process to request l10n support.  Adding needinfo? :l10n.

Comment 5

[Axel Hecht [:Pike]]

See https://groups.google.com/forum/#!topic/mozilla.dev.gaia/SrFrWrBNl20 for instructions, we should really try to not take this.

Comment 6

[Stephany Wilkes]

UX is still giving priority to blocking bugs. Assigning to Rob for advice on strings while noting that this is late-l10n and not yet a blocker.

Comment 7

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Empty dialog screenshot.

Copy screenshot from bug 883341 to help release drivers.

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

To help triage, note that this bug was derived from bug 883341 which is already leo+.

Comment 9

[Francisco Jordano [:arcturus] [:francisco]]

+1 to Ben,

this is the patch for to save contacts to suffer from bug 883341 that is already a leo+.

Thanks!

Comment 10

[Francisco Jordano [:arcturus] [:francisco]]

(Don't know why my previous comment removed the needinfo :S)

Already made some comments in the code about things we are missing.

Also another idea came to my mind:

Right now we launch contacts as an activity and load the contacts list, wait in it, and when the user clicks on a contact we display that message.

Another alternative is doing this control here:

https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/contacts/js/activities.js#L56

sanitise the pick activity info needed, and if we don't have the proper filters display the alert that we are showing in ths PR. With that, the user will get feedback sooner, and wont need to click in a contact to know the contacts app is not going to handle the request. If we decide to go for this option we should still modify the code so the dataPickHandler function is safe anyway.

wdyt folks?

Comment 11

[:gerard-majax]

Latest developments on bug 883341 are making this one not really important now. I'm not sure it should be leo+: the fix for 883341 seems to be okay as far as I've tested, it avoid any access to Contacts from the described case, hence the user cannot get trapped in the present bug.

Comment 12

[Ben Kelly [:bkelly, not reviewing]]

Agree.  The fix for bug 883341 looks good and removes the urgency for fixing this.  I'll go ahead and remove my leo? nom and the late-l10n flag.

Leaving this open, though, since its probably a good idea to do something other than the blank dialog with the unexpected activity type.  Now that we have required=true in our filters, perhaps we should immediately postCancel() or even throw()?  Thoughts?

Comment 13

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 768955 [details]
Link to Github https://github.com/mozilla-b2g/gaia/pull/10700

As the main problem has been fixed in bug 883341, this PR is not needed anymore, but IMHO, we should modify  this part in order to not to have any possible case where we display an empty dialog.

In this case the wording I think should change.

Thanks a lot guys for the work to solve this!

Comment 14

[Ben Kelly [:bkelly, not reviewing]]

This hasn't had any activity in the last week.  Can we mark this WONTFIX since bug 883341 should prevent this situation from happening?

Comment 15

[Francisco Jordano [:arcturus] [:francisco]]

As per comment 14