[Ringtones] Migrate to tagged template strings

RESOLVED FIXED in 2.2 S13 (29may)

Status

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: kgrandon, Assigned: kgrandon)

Tracking

unspecified
2.2 S13 (29may)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [systemsfe])

Attachments

(1 attachment)

Assignee

Description

4 years ago
For security reasons and consistency we are porting all template.js usage over to our tagged template string handling.
Assignee

Comment 2

4 years ago
Comment on attachment 8606216 [details] [review]
[gaia] KevinGrandon:bug_1165257_ringtones_template_strings > mozilla-b2g:master

Jim or David - either of you guys have cycles to review this? Thanks!
Attachment #8606216 - Flags: review?(squibblyflabbetydoo)
Attachment #8606216 - Flags: review?(dflanagan)
Wouldn't it make more sense to replace things like this with web components? I thought that was the whole point of web components.
Assignee

Comment 4

4 years ago
(In reply to Jim Porter (:squib) from comment #3)
> Wouldn't it make more sense to replace things like this with web components?
> I thought that was the whole point of web components.

That's potentially an option, though even so we would still need to sanitize data and pass it into the web component. Instead of doing that with template.js, we would probably continue to use the tagged template string library for this. So I view this as working well with web components in the future, and a stepping stone to get to a better place.
Also, wow. I looked at the MDN page about JS template strings to figure out what was going on in the syntax and it's like someone decided to weaponize Perl.
Comment on attachment 8606216 [details] [review]
[gaia] KevinGrandon:bug_1165257_ringtones_template_strings > mozilla-b2g:master

r- for now, just because I'd rather not use the "module pattern" in the ringtones app, especially not for cases where we're really just setting a global variable to an object. Otherwise though, this looks good (inasmuch as anything using this syntax can be said to look "good").
Attachment #8606216 - Flags: review?(squibblyflabbetydoo) → review-
Comment on attachment 8606216 [details] [review]
[gaia] KevinGrandon:bug_1165257_ringtones_template_strings > mozilla-b2g:master

Clearing the review request for me. Jim is the ringtones owner.
Attachment #8606216 - Flags: review?(dflanagan)
Assignee

Comment 8

4 years ago
Comment on attachment 8606216 [details] [review]
[gaia] KevinGrandon:bug_1165257_ringtones_template_strings > mozilla-b2g:master

Hi Jim - I've addressed your review comments. Please take a look again when you have a chance. Thanks!
Attachment #8606216 - Flags: review- → review?(squibblyflabbetydoo)

Updated

4 years ago
Attachment #8606216 - Flags: review?(squibblyflabbetydoo) → review+
Assignee

Comment 9

4 years ago
Thanks for the review!
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Whiteboard: [systemsfe]
Target Milestone: --- → 2.2 S13 (29may)
You need to log in before you can comment on or make changes to this bug.