Closed Bug 1253120 Opened 4 years ago Closed 4 years ago

Allow UITour's showFirefoxAccounts command to allow non alphanumeric strings for param values

Categories

(Firefox :: Sync, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 47
Tracking Status
firefox47 --- fixed

People

(Reporter: markh, Assigned: markh)

References

Details

Attachments

(1 file)

It turns out that the restriction on param values we introduced in bug 1244929 is too restrictive, and given the code will auto-encode values, there's no good reason to keep that restriction.

This patch keeps the existing restriction in param names, but allows any characters in the param values. The test ensures these values are encoded in the final URL.
Attachment #8725996 - Flags: review?(MattN+bmo)
Comment on attachment 8725996 [details] [diff] [review]
0001-Bug-XXXXXXX-drop-the-restriction-on-param-values-pas.patch

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

Sorry about causing the churn by making this extra strict. I hadn't seen paths used in the utm parameters before
Attachment #8725996 - Flags: review?(MattN+bmo) → review+
https://hg.mozilla.org/mozilla-central/rev/2eb7b3fd4cc0
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Just a note that the comment in UITour-lib.js [1] still states that values must be alphanumeric. Probably good to clean that up to avoid future confusion. :)

(Please ignore if this is already done/underway and just not yet reflected.)

[1] https://dxr.mozilla.org/mozilla-central/source/browser/components/uitour/UITour-lib.js#240
Thanks, I just pushed a followup
You need to log in before you can comment on or make changes to this bug.