Bug 194620 - Ignore Ad Click Attribution where source and destination are same-site
Summary: Ignore Ad Click Attribution where source and destination are same-site
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: John Wilander
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-02-13 15:50 PST by John Wilander
Modified: 2019-02-13 17:36 PST (History)
8 users (show)

See Also:


Attachments
Patch (9.09 KB, patch)
2019-02-13 15:53 PST, John Wilander
no flags Details | Formatted Diff | Diff
Patch for landing (9.59 KB, patch)
2019-02-13 16:58 PST, John Wilander
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Wilander 2019-02-13 15:50:19 PST
We should not accept Ad Click Attributions where the current website and the addestination attribute are same-site. The site doesn't need attributions sent for itself.
Comment 1 John Wilander 2019-02-13 15:50:34 PST
<rdar://problem/47890018>
Comment 2 John Wilander 2019-02-13 15:53:39 PST
Created attachment 361950 [details]
Patch
Comment 3 Jiewen Tan 2019-02-13 16:09:56 PST
Comment on attachment 361950 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=361950&action=review

LGTM except for the lack of the clarifications. John explained that to me personally. Please add those explanations to the change log before landing. r=me.

> Source/WebCore/ChangeLog:9
> +        Updated existing test.

I guess some clarifications of the necessity of this patch is needed here.

> Source/WebCore/html/HTMLAnchorElement.cpp:449
> +    if (documentDomain == adDestinationHost) {

It is a bit unfortunate that platforms that don't have PUBLIC_SUFFIX_LIST get different behaviors.
Comment 4 John Wilander 2019-02-13 16:18:56 PST
(In reply to Jiewen Tan from comment #3)
> Comment on attachment 361950 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=361950&action=review
> 
> LGTM except for the lack of the clarifications. John explained that to me
> personally. Please add those explanations to the change log before landing.

Good idea. Will do.

> r=me.
> 
> > Source/WebCore/ChangeLog:9
> > +        Updated existing test.
> 
> I guess some clarifications of the necessity of this patch is needed here.

Yup.

> > Source/WebCore/html/HTMLAnchorElement.cpp:449
> > +    if (documentDomain == adDestinationHost) {
> 
> It is a bit unfortunate that platforms that don't have PUBLIC_SUFFIX_LIST
> get different behaviors.

Yes. I'll make a note of that in the change log too.

Thanks for the review, Jiewen!
Comment 5 John Wilander 2019-02-13 16:19:24 PST
I will wait for the bots to go green before landing.
Comment 6 John Wilander 2019-02-13 16:58:36 PST
Created attachment 361967 [details]
Patch for landing
Comment 7 WebKit Commit Bot 2019-02-13 17:36:33 PST
Comment on attachment 361967 [details]
Patch for landing

Clearing flags on attachment: 361967

Committed r241490: <https://trac.webkit.org/changeset/241490>
Comment 8 WebKit Commit Bot 2019-02-13 17:36:35 PST
All reviewed patches have been landed.  Closing bug.