Add support for specifying a custom camera app for image questions

[lognaturel]

In support of https://github.com/getodk/collect/issues/5621

In the parameters column for image questions, introduce an app key for fields of type image. The value of this key should be put in a body attribute with name intent.

Example: app=com.jeyluta.timestampcamerafree

This is very similar to the existing intent body attribute used to specify an external app that returns multiple values: https://github.com/XLSForm/pyxform/blob/129abe98c97025cb5b978125e5d2ba5c0c0f74f7/pyxform/section.py#L191, https://docs.getodk.org/launch-apps-from-collect/#external-apps-to-populate-multiple-fields

The value is an Android application id. My understanding is that these must always use Java package name conventions so we could do some validation there, e.g. using regex. I'd like someone else to confirm that the structure is guaranteed as part of deciding whether not we validate.

[lindsay-stevens]

From the linked Collect discussion and the docs, it seems like it's possible to specify a custom image capture app already, using appearance or body::intent. These column names maybe aren't the best but it's an established method (in the docs and user's forms) and it supports many data types for single questions and groups of questions. So the following would be equivalent:

Current method, which can also pass parameters to the external app:

| survey |        |          |       |                                    |
|        | type   | name     | label | appearance                         |
|        | image  | my_image | Image | ex:com.jeyluta.timestampcamerafree |

Proposed alternative, which would only specify the image capture app to open:

| survey |        |          |       |                                     |
|        | type   | name     | label | parameters                          |
|        | image  | my_image | Image | app=com.jeyluta.timestampcamerafree |

And for groups, users would still need to do it this way:

| survey |             |         |         |            | body::intent                    |
|        | type        | name    | label   | appearance | com.jeyluta.timestampcamerafree |
|        | begin_group | mygroup | Images  | field-list |                                 | 
|        | image       | image1  | Image 1 |            |                                 |
|        | image       | image2  | Image 2 |            |                                 |
|        | end_group   |         |         |            |                                 |

Not unique to this parameter but what does Collect do when users specify conflicting info, like this?

| survey |             |         |         |                  |            | body::intent                    |
|        | type        | name    | label   | parameters       | appearance | com.jeyluta.timestampcamerafree |
|        | begin_group | mygroup | Images  |                  | field-list |                                 | 
|        | image       | image1  | Image 1 | app=org.otherapp |            |                                 |
|        | image       | image2  | Image 2 |                  |            |                                 |
|        | end_group   |         |         |                  |            |                                 |

Shortcuts can be useful but is this an clear win? The costs of shortcuts are possible confusion from having multiple ways to do the same thing, often with reduced flexibility (DP 1,2), and ongoing software/docs maintenance (DP 2). The former was a theme at the XLSForm / XPath session at the summit this year. For the latter I am thinking of #592 and the large feature space in pyxform.

[grzesiek2010]

it seems like it's possible to specify a custom image capture app already, using appearance

Correct but this was implemented for so called external apps (not necessarily even camera apps) plus using the appearance column to achieve that doesn't seem right (that column should be used for specifying visual not behavioral differences in questions). I think we should rather deprecate it and use parameters in this case as well (but that's later after fixing this issue)

body::intent on the other hand was only added to support populating multiple fields see https://docs.getodk.org/launch-apps-from-collect/#external-apps-to-populate-multiple-fields why not use parameters here too. Having a separate column that does something only for groups doesn't make a lot of sense.

We discussed both those options with @lognaturel not only in the issue (what you can read) but also during a 1on1 meeting and we like the option with parameters more.

[grzesiek2010]

Not unique to this parameter but what does Collect do when users specify conflicting info, like this?

I think in this case the group level intent should be more important. I'm not 100% sure now but I think questions in a group with intent are read-only in a way that they can by populated by an external app but buttons/text fields in the questions are hidden or disabled and you can't add answers manually. So If it's one group it doesn't make sense to support different intents for different questions that are inside.

[lognaturel]

Current method, which can also pass parameters to the external app:

Using an appearance with ex: prefix launches an external app as you say, but the external app needs to know to return its value back to Collect. It's generally intended to be used with custom apps that specifically integrate with Collect. This new functionality we've added to Collect is to specify a standard camera app to be used just with the image question type.

I guess one thing I hadn't thought about is that we could probably have extended the existing ex: functionality for images instead of adding an additional attribute. So Collect could first try calling the app with the IMAGE_CAPTURE intent and if that fails launch it as we currently do external apps. @grzesiek2010 I don't think we discussed this, right? When we were considering using the existing ex: syntax I think I had forgotten that custom apps could return single images as ClipData so the ex: appearance can already be used with image questions. The downsides are that as @grzesiek2010 we really don't like using the appearance attribute to pass things in that aren't display hints and the ex: prefix is hard to remember. We'd be extending the use of a pattern we generally don't like. But the positive would be that specifying a custom app to get an image would be the same as specifying a standard camera app.

why not use parameters here too

@grzesiek2010 I think we did discuss eventually also adding the app= parameter for body::intent for groups. That would at least make those two consistent.

The costs of shortcuts are possible confusion from having multiple ways to do the same thing

Yes, definitely something to be very mindful of. My sense is that the body::, bind:: generic XLSForm mappings to XForm attributes are generally intended to be aliased if they become broadly useful. The existing external app functionality is pretty niche because it generally requires a custom app. This new concept we've introduced has the potential to be much more approachable: you can use it to specify any Android camera app. That made it feel worth having a more friendly shortcut to me.

[grzesiek2010]

There is one thing that only now came to my mind... If we use external apps we should be able to set custom error message if such an app does not exist. We can do that using noAppErrorString column in xls. It would be good to have the same for custom camera apps. Of course we could add noAppErrorString as a new parameter too but then it would need to store raw strings. It works like that either way we can't add multiple translations for noAppErrorString but at some point maybe we would like to do that. If we go for parameters it won't be doable. I still think that using appearance column is the worst option but I've changed my mind a little bit regarding the intent column. Theoretically every single question type could support that column and give a possibility to start external apps, plus we would have a separate (translatable) column for noAppErrorString. @lognaturel what do you think about noAppErrorString? I'm not sure how important it is. If the app is not available our error message seems to be clear enough.

[lognaturel]

If we use external apps we should be able to set custom error message if such an app does not exist. We can do that using noAppErrorString column in xls.

I had also forgotten about this! I don't see it in any documentation and I doubt it's in broad use. It seems it would be easier to put instructions for downloading the app as part of a label or hint. This doesn't change my thinking around how we specify the app to launch.

Here's a summary of when I think it's valuable to use the parameters column:

• The values specified are important to understanding how that form row behaves. If so, it's nice to keep those values as close to the type/name/label as possible which is better done with an existing column (parameters). Here, I think keeping in mind that a custom app will be launched when requesting a picture is valuable. Similarly, with range questions, the parameters are critical to understanding what the row will do.
• There's likely to be very few of these values specified in a single form. If so, a new column would be almost empty, making it easy to miss rows that are populated. For this feature, I'd be really surprised if a single form launched a special camera app more than once or twice. Similarly, we use parameters for specifying custom value and label values for selects from files. Usually there aren't many of those in a given form.
• The functionality is unlikely to appear in many forms. That means we wouldn't want a specialized column to be part of a standard template. In this case, we know not that many forms include media capture at all and we expect this to be rarer yet. Using parameters means it can be specified in a standard template.
• The functionality is not exceedingly rare/doesn't require special technical knowledge. This is the flipside of the point above. If functionality is really truly very niche, then those advanced users who need it probably don't mind adding a column. I think noAppErrorString falls in that category. I don't feel like we should expose it any further but we can keep it around for those who really need it.

All of these have me continuing to feel like the solution here is a good one! If you all agree, let's get it to completion.

@lindsay-stevens let's also work on getting some criteria like this around use of the parameters column into the readme. I'd be very interested in seeing how aligned we are on them.

[lindsay-stevens]

• Is the app parameter intended to work with the existing max-pixels parameter? (if so it needs a test).
• If a user specifies a valid app name but it's not installed on the device, what is the error message from Collect and is it localised?

I've linked the parameters comments to the existing ticket for that. I think parameters can be a useful shortcut when used sparingly, but they have major limitations in relation to parsing values (e.g. multiple or complex params) and specifying translations.

[lognaturel]

Is the app parameter intended to work with the existing max-pixels parameter? (if so it needs a test).

@grzesiek2010 I think so? Is it tested in Collect, too?

what is the error message from Collect and is it localised

"No activity found to handle: %s" and yes, it's translated.

multiple or complex params) and specifying translations.

Yes, agreed!

[grzesiek2010]

Is the app parameter intended to work with the existing max-pixels parameter? (if so it needs a test).

I've added a test for that case.

@grzesiek2010 I think so? Is it tested in Collect, too?

No, in Collect we don't have end-to-end tests for max-pixels too. We only test the class that is responsible for handling compression in isolation. It will be verified by the QA team for sure though. I'm not sure if having such a test in Collect is a must, it would be nice so maybe I will think about it or just file a separate issue.