[Web Activities] Activities filtering heuristic does not support arrays in filters

RESOLVED FIXED in mozilla17

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: vingtetun, Assigned: fabrice)

Tracking

Trunk
mozilla17
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Blocks: 776027
(Assignee)

Updated

5 years ago
Duplicate of this bug: 782755
(Assignee)

Comment 2

5 years ago
Created attachment 652156 [details] [diff] [review]
patch

This patch provides support for arrays. I'm not sure we can do regexps since they are not serializable in the json manifest (I very well may be wrong on this).

djf tested the patch with the gallery app.
Assignee: nobody → fabrice
Attachment #652156 - Flags: review?(21)
Comment on attachment 652156 [details] [diff] [review]
patch

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

::: dom/activities/src/ActivitiesService.jsm
@@ +254,5 @@
> +        if (Array.isArray(aResult.description.filters[prop])) {
> +          if (aResult.description.filters[prop].indexOf(aMsg.options.data[prop]) == -1) {
> +            return false;
> +          }
> +        } else if (aMsg.options.data[prop] !== aResult.description.filters[prop]) {

nit: For consistency I would invert the last expression so you can read it the same order as the indexOf expression.
Attachment #652156 - Flags: review?(21) → review+
(Assignee)

Comment 4

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/31036bc023a8

Comment 5

5 years ago
https://hg.mozilla.org/mozilla-central/rev/31036bc023a8
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Fabrice, can you open a follow-up to support regexps, if we don't have a bug on file yet?
Summary: [Web Activities] Activities filtering heuristic does not support arrays/regexp in filters → [Web Activities] Activities filtering heuristic does not support arrays in filters
I think Fabrice is right in comment #2.  You can't use regexp literals in JSON.  So you'll need to pick a syntax to distinguish a regexp string from just a regular literal string.

Could you get away with using ordinary wildcards?

pattern = new RegExp(s.replace('*', '\S*'))

Or something like that?

Updated

5 years ago
QA Contact: jsmith
Whiteboard: [qa+]
(In reply to David Flanagan [:djf] from comment #7)
> I think Fabrice is right in comment #2.  You can't use regexp literals in
> JSON.  So you'll need to pick a syntax to distinguish a regexp string from
> just a regular literal string.
> 
> Could you get away with using ordinary wildcards?
> 
> pattern = new RegExp(s.replace('*', '\S*'))
> 
> Or something like that?

We should not use JSON but structured clones.
(Assignee)

Comment 9

5 years ago
(In reply to Mounir Lamouri (:mounir) from comment #8)
> 
> We should not use JSON but structured clones.

We still need a way to write that in the manifest. How do structured clones help here?
(In reply to Fabrice Desré [:fabrice] from comment #9)
> (In reply to Mounir Lamouri (:mounir) from comment #8)
> > 
> > We should not use JSON but structured clones.
> 
> We still need a way to write that in the manifest. How do structured clones
> help here?

Indeed. I don't have any answer regarding this for the moment.

Updated

5 years ago
Keywords: verifyme
Whiteboard: [qa+]

Updated

5 years ago
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.