Remove FXIOS-15286 [Telemetry] Ad cookies parsing not used#32832
Remove FXIOS-15286 [Telemetry] Ad cookies parsing not used#32832issammani merged 2 commits intomozilla-mobile:mainfrom
Conversation
janbrasna
left a comment
janbrasna
left a comment
There was a problem hiding this comment.
Before its PR even landed, it seems the only cookie parsing listed for bing eventually got removed in c144c34a within #8970 before the feature shipped. Nothing has shown since to be adding any use of this specific message data, so I'm assuming the collection on the document side is completely superfluous. @nbhasin2 Does that sound plausible from when this functionality was launched?
| 'urls': getLinks(), | ||
| 'cookies': getCookies() | ||
| 'urls': getLinks() | ||
| }; | ||
|
|
||
| webkit.messageHandlers.adsMessageHandler.postMessage(message); |
There was a problem hiding this comment.
Looking at the adsMessageHandler processing it, I only ever see iterating over the links:
firefox-ios/firefox-ios/Client/Telemetry/AdsTelemetryHelper.swift
Lines 119 to 120 in 8308f2f
but no cookie info (from message
body["cookies"]) is ever parsed.
It also appears the whole message payload is never stored raw, so that would hint at only parsing the explicit values, which is currently just the links.
🧹 Tidy commitJust 2 file(s) touched. Thanks for keeping it clean and review-friendly! ✅ New file code coverageNo new file detected so code coverage gate wasn't ran. Generated by 🚫 Danger Swift against 7669339 |
|
This PR has been automatically marked as stale. Please leave any comment to keep this PR opened. It will be closed automatically if no further update occurs in the next 7 days. Thank you for your contributions! |
github-actions
Bot
added
the
stale
github-actions
Bot
removed
the
stale
lmarceau
left a comment
lmarceau
left a comment
There was a problem hiding this comment.
LGTM, should we confirm with Nish/Issam before merging? Just want to know was there any offline updates on this conversation?
-- Its possible and we don't use cookie for parsing so this is more like a bloat cc @issammani thoughts? |
|
This looks good to me. I don't see anything in history that suggests we were ever collecting cookies. Thanks @janbrasna for looking into this ❤️ |
|
🚀 PR merged to |
5 participants
📜 Tickets
Jira ticket FXIOS-15286
GitHub issue #32826
💡 Description
The values collected JS–side are not utilized further by the native helpers. This has caused WebKit crashes so it would be better to remove completely if unused.
📝 Checklist