Closed Bug 1073501 Opened 7 years ago Closed 7 years ago

Include activity type in activity-choice mozChromeEvent

Categories

(Firefox OS Graveyard :: Runtime, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
2.1 S8 (7Nov)

People

(Reporter: davidg, Assigned: davidg)

References

Details

Attachments

(1 file, 2 obsolete files)

When gecko triggers an 'activity-choice' mozChromeEvent, the activity name is passed to gaia (for example: 'open'), but not the activity type (for example: 'text/vcard').
In order to be able to remember what application was selected by the user for that particular activity type (bug 1039386), we should pass the activity type in that event as well.
Assignee: nobody → david.garciaparedes
Blocks: 1073495
Attached patch 1073501.patch (obsolete) — Splinter Review
Attachment #8513506 - Flags: review?(fabrice)
Comment on attachment 8513506 [details] [diff] [review]
1073501.patch

Review of attachment 8513506 [details] [diff] [review]:
-----------------------------------------------------------------

In this patch you send much more than the type, and that's potentially a lot of data (eg memory blobs). What do you need exactly?
Attachment #8513506 - Flags: review?(fabrice) → review-
We just need the type. We need it because we want to be able to remember which application was chosen by the user, for a particular activity_name+type combination.
The reason I am sending the whole data object, it is because this code seems to be agnostic about the contents of the data object, and I wasn't sure if checking for type inside data was maybe too specific. Also I thought maybe the rest of the data object could be useful for other things in the future. But you are right, it could potentially be a lot of data.

So maybe what I can do is; check in the ActivitiesGlue if the data object contains a type, and if it does, I just send the type in the activity-choice message. What do you think?
Flags: needinfo?(fabrice)
Attached patch v2 (obsolete) — Splinter Review
Second version of the patch, just attaching the type in case it is defined inside data.
Attachment #8514324 - Flags: review?(fabrice)
Comment on attachment 8514324 [details] [diff] [review]
v2

Review of attachment 8514324 [details] [diff] [review]:
-----------------------------------------------------------------

If it's good enough for gaia, let's do that.
One question though: Once I have set "remember me" on share image/png, what happens if I install a new app that also provides this activity? Ideally that should disable the 'remember me' for this situation.

::: b2g/components/ActivitiesGlue.js
@@ +66,5 @@
>      if (aActivities.length === 0) {
>        aCallback.handleEvent(Ci.nsIActivityUIGlueCallback.WEBAPPS_ACTIVITY, -1);
>        return;
>      }
> +    

nit: trailing whitespace.
Attachment #8514324 - Flags: review?(fabrice) → review+
Flags: needinfo?(fabrice)
Yes. If you install an app that provides the same activity, it should reset it, and the user will be ask again.
There is also going to be a menu in settings to change your selection.
You can have a look at the UX spec in bug 1039386

Thanks for the review!
Attached patch v2 + nitSplinter Review
r=fabrice
Attachment #8513506 - Attachment is obsolete: true
Attachment #8514324 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/cb033d870ecd
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S8 (7Nov)
Blocks: 1101425
Blocks: 1141479
You need to log in before you can comment on or make changes to this bug.