Closed
Bug 773383
Opened 12 years ago
Closed 12 years ago
[Web Activities] Activities filtering heuristic does not support arrays in filters
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla17
People
(Reporter: vingtetun, Assigned: fabrice)
References
Details
Attachments
(1 file)
1.27 KB,
patch
|
vingtetun
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 2•12 years ago
|
||
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)
Reporter | ||
Comment 3•12 years ago
|
||
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•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/31036bc023a8
Comment 5•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/31036bc023a8
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Comment 6•12 years ago
|
||
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
Comment 7•12 years ago
|
||
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•12 years ago
|
QA Contact: jsmith
Whiteboard: [qa+]
Comment 8•12 years ago
|
||
(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•12 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?
Comment 10•12 years ago
|
||
(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
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•