Closed Bug 1511156 Opened Last year Closed 11 months ago

windows attribution data is double encoded

Categories

(Firefox :: Messaging System, enhancement, P1)

65 Branch
enhancement

Tracking

()

RESOLVED FIXED
Firefox 66
Iteration:
66.1 - Dec 10-23
Tracking Status
firefox65 --- wontfix
firefox66 --- fixed

People

(Reporter: mixedpuppy, Assigned: andreio, NeedInfo)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

We will need to double decode the data in order for about:welcome to work properly.  See bug 1511071
Specifically, when AMO passes an extension id in utm_content, we end up with data that looks like:

campaign%3Dnon-fx-button%26content%3DtextMarker%2540underFlyingBirches.org%26medium%3Dreferral%26source%3Daddons.mozilla.org

Note the %2540, which is a double encoded @.  The scratchpad test in attachment 9028744 [details] deals with it by continuing to decode until no more % exists, but that's not great (was quick and dirty).
Hi Kate, How do we flag this as a P1 blocker for RTAMO feature?  We can't do the QA testing from bug 1511071 for Windows without this fix. 

You mentioned it was likely a quick fix to land in Nightly, QA can test end-to-end and if it passes we would request early Beta 65 uplift for this.
Flags: needinfo?(khudson)
Assignee: nobody → andrei.br92
Flags: needinfo?(khudson)
Iteration: --- → 66.1 - Dec 10-23
Priority: -- → P1
mhowell, is it intended that `AttributionCode.getAttrDataAsync` returns double encoded for windows consumers to decide if they should decode with something like:

    while (content.includes("%")) {
      content = decodeURIComponent(content);
Flags: needinfo?(mhowell)
It's expected, yes. There's been discussion about changing that over in bug 1487767, but it sounded like fixing it universally would have some unintended consequences.
Flags: needinfo?(mhowell)
Okay thanks. I would hope that we can just assume the value on windows is encoded consistently and do a single decodeURIComponent? E.g., some @ in an addon id should be "%40"
See Also: → 1487767
Yes, URI-encoding is all that is happening there.
[github robot] where are youuu?

https://github.com/mozilla/activity-stream/commit/577aa3fd672c39137deb59361c3e1293d10fffcd
Status: NEW → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Blocks: 1515206
https://hg.mozilla.org/mozilla-central/rev/e1439f07a4a4
Target Milestone: --- → Firefox 66
Is this something we should consider uplifting to Beta or can it ride the trains?
Flags: needinfo?(andrei.br92)
This feature has been deprioritized for 66 it can ride the trains, thanks.
Flags: needinfo?(andrei.br92)
In order to verify this we need to have the feature working on Windows first

The feature has landed on Windows but the steps to verify this are not very clear so far.

Flags: needinfo?(andrei.br92)

Shane, can you help us with some info on how to validate this part?

Flags: needinfo?(mixedpuppy)

The attribution data from a windows download will be double encoded, so you should be fine testing with data from that.

Flags: needinfo?(mixedpuppy)
Component: Activity Streams: Newtab → Messaging System
You need to log in before you can comment on or make changes to this bug.