951104 - Enable <a ping> by default

Status: NEW

(Core :: DOM: Navigation, enhancement)

Description

[Tim Taubert [:ttaubert] (inactive)]

We currently ship with browser.send_pings=false and browser.send_pings.max_per_link=1.

When enabling pings by default we should probably also look into raising or disabling the max_per_link limit, Blink (as of now) does not restrict the number of pings.

We should also find out whether bug 401217 and bug 401352 are valid blockers.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Before enabling ping, better to check our implementation matches the spec.
IIRC, but could be wrong, our ping implementation follows some early version of the spec, and
it changed significantly later.

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Olli Pettay [:smaug] from comment #1)
> Before enabling ping, better to check our implementation matches the spec.
> IIRC, but could be wrong, our ping implementation follows some early version
> of the spec, and
> it changed significantly later.

Yes :) I filed this bug just after I pushed bug 786347.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Do we need to bump up "browser.send_pings.max_per_link"? I'd consider leaving it at 1 and see what affect it has. On the other hand, I wanted to look into setting "browser.send_pings.require_same_host" to "true" to limit the privacy fallout that could happen if we turn on <a ping>.

I'd like to see this turned on for mobile because sites that use redirection to track link clicks can switch to <a ping> and improve pageload times. Google is already doing this for Chrome and Safari:
https://plus.google.com/+IlyaGrigorik/posts/fPJNzUf76Nx

Comment 4

[Doug Turner (:dougt)]

navigator.sendBeacon is also a google/ms supported API that solves similar usecases.

Jonas, I think based on our twitter exchange, https://twitter.com/sickingj/status/413560345974693889, I think may want to mark this wont-fix with a follow up to remove the code from m-c to avoid further confusion.

Tim, if you want to own sendBeacon() that would be great.  It is in bug 936340.  If so, I can wrap up my latest changes and update the bug.

Comment 5

[Andrea Giammarchi]

> I'd like to see this turned on for mobile because sites that use redirection to track link clicks can switch to <a ping> and improve pageload times.

You should ask permission to those paying for the mobile bandwidth first ... keep it optional or enable by default only in wifi conditions or if the carrier tells you it's OK to do so

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Andrea Giammarchi from comment #5)
> > I'd like to see this turned on for mobile because sites that use redirection to track link clicks can switch to <a ping> and improve pageload times.
> 
> You should ask permission to those paying for the mobile bandwidth first ...
> keep it optional or enable by default only in wifi conditions or if the
> carrier tells you it's OK to do so

You are already paying the bandwidth. Google (and other sites that track clicks) are making two pageloads.

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Doug Turner (:dougt) from comment #4)
> navigator.sendBeacon is also a google/ms supported API that solves similar
> usecases.
> 
> Jonas, I think based on our twitter exchange,
> https://twitter.com/sickingj/status/413560345974693889, I think may want to
> mark this wont-fix with a follow up to remove the code from m-c to avoid
> further confusion.

Why? <a ping> is already being used in the field. How soon before sendBeacon() is supported well enough to be valuable?

Comment 8

[Doug Turner (:dougt)]

We can probably support it in ff29.  The answer to your other question is not answerable.

Comment 9

[Doug Turner (:dougt)]

(oh keep in mind, the spec is in flux, so there is that)...

Comment 10

[Andrea Giammarchi]

(In reply to Mark Finkle (:mfinkle) from comment #6)
> You are already paying the bandwidth. Google (and other sites that track
> clicks) are making two pageloads.

I don't follow your logic ... Google waste bandwidth, let's everybody follow?

I am not sure the impact of a "ping" but if it downloads things behind the scene you should **really** keep it optional or inform users about this.

I thought FFOS was aiming to reach also developing markets where contracts with 5GB per month are not the reality.

Just saying ...

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Mark Finkle (:mfinkle) from comment #7)
> (In reply to Doug Turner (:dougt) from comment #4)
> > navigator.sendBeacon is also a google/ms supported API that solves similar
> > usecases.
> > 
> > Jonas, I think based on our twitter exchange,
> > https://twitter.com/sickingj/status/413560345974693889, I think may want to
> > mark this wont-fix with a follow up to remove the code from m-c to avoid
> > further confusion.
> 
> Why? <a ping> is already being used in the field. How soon before
> sendBeacon() is supported well enough to be valuable?

It looks like sendBeacon() supports a different use case, but an important one:
https://dvcs.w3.org/hg/webperf/raw-file/tip/specs/Beacon/Overview.html

Comment 12

[Doug Turner (:dougt)]

@andreas, this is the wrong place to have a discussion like this.  please fine a forum and keep this bug techincal in nature.

Comment 13

[Andrea Giammarchi]

OK, I was rather pointing out this might not be consider a bug. Best Regards

Comment 14

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

> Why? <a ping> is already being used in the field. How soon before
> sendBeacon() is supported well enough to be valuable?

As far as I know, only the main google search page uses <a ping>. We've checked with them and they would deploy sendBeacon if we implement it.

> It looks like sendBeacon() supports a different use case, but an important
> one:
> https://dvcs.w3.org/hg/webperf/raw-file/tip/specs/Beacon/Overview.html

As far as I can tell sendBeacon supports all the use cases of <a ping> as well as additional use cases. I.e. sendBeacon supports a superset of use cases, not a different set.

The only thing that <a ping> brings is that it can be done without scripting. Which honestly isn't a strong use case to me. Especially for a feature like this.

Comment 15

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Adding info that I posted elsewhere as to keep all the discussion in one place:

As far as I can tell, the following is true:

1. sendBeacon is a superset of <a ping> functionality-wise
2. The only thing <a ping> brings over sendBeacon is that <a ping>
doesn't require JS.
3. If MS+Google implement sendBeacon we'll likely be forced to
implement no matter what we do for <a ping>.
4. Even a naive implementation of sendBeacon is more work to
implement, but most of the <a ping> implementation can be leveraged.

So my money is on implementing sendBeacon. Mostly because I think in the end we'll be forced to anyway. And it'll improve performance in more situations which is a good thing.

Comment 16

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Jonas Sicking (:sicking) needinfo? or feedback? encouraged. from comment #15)
> 2. The only thing <a ping> brings over sendBeacon is that <a ping>
> doesn't require JS.

The advantage of <a ping> is timeliness. We could enable <a ping> in Firefox 29 with no trouble. It isn't clear, because of the status of the spec, when we could enable sendBeacon. Assuming we had a working sendBeacon with tests in January, would we enable sendBeacon in Firefox 29? And, if not, when would we enable it?

Otherwise, I don't care.

Comment 17

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I believe we can stabilize the spec very quickly. It hasn't changed dramatically since the initial draft and there's currently no outstanding comments to it.

So yes, I believe we can have a stable spec in order for us to ship with Gecko 29.

Comment 18

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Brian Smith (:briansmith, was :bsmith; Please NEEDINFO? me if you want a response) from comment #16) 
> The advantage of <a ping> is timeliness. We could enable <a ping> in Firefox
> 29 with no trouble.
Well, someone should implement spec-compliant <a ping> first. The current implementation has been
unused for years, and IIRC, the spec changed after its implementation.

Comment 19

[Doug Turner (:dougt)]

we don't want to support both, if we have a choice.  sendBeacon provides what looks to be a superset of functionality.  We have beacon working code (lots of todo's!).  Sicking is pushing on the spec hard.

I believe this bug should be WONTFIX.

Comment 20

[Karl Dubost💡 :karlcow]

*IF* the decision was "enabling ping by default", I would put this as a blocker: Bug 401352 

At http://www.whatwg.org/specs/web-apps/current-work/#hyperlink-auditing
> When the ping attribute is present, user agents 
> should clearly indicate to the user that following 
> the hyperlink will also cause secondary requests 
> to be sent in the background, possibly including 
> listing the actual target URLs.

It's a SHOULD, but in the context of Firefox being a more privacy aware browser, it might become an important SHOULD.

Comment 21

[Doug Turner (:dougt)]

I am okay if we ignore me in comment 19.

Comment 22

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0004-Bug-951104-Enable-a-ping-by-default.patch

Per discussion on dev.platform I think we want to land this without any UI, this can indeed be explored by add-ons. We don't have a great place to show ping attribute values right now and it's very easy to experiment with that.

We don't limit the number of pings per link and allow contacting hosts that are not the same host as the document that contains the link being clicked. Both of these setting can be changed in about:config, the whole feature can be turned off as well.

Parking this patch here until its dependencies are resolved.

Comment 23

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Was there actually any decision in the dev.platform thread? I'm still not clear on why we want to ship this feature?

Comment 24

[Karl Dubost💡 :karlcow]

For the paper trail. The thread on dev.platform is at
https://groups.google.com/forum/#!topic/mozilla.dev.platform/DxvZVnc8rfo

* Tim Taubert posted an intent to ship on May 16, 2014
* The discussion has been mainly about having a preference to allow 
  or not allow the ping.
* Arguments have been that if there is a preference to block them, 
  Web properties will fallback on the old way and defeat the purpose 
  of the preference. 
* Some posters have made the point that Firefox being a privacy aware 
  browser, not giving the choice to users is not good. Someone asked 
  if the Private Tab would allow or disallow the ping. 
* The current practice is redirecting users through a proxy for tracking 
  them. Someone argued that "Yes; but the methods being used by at (least 
  some major) sites are visible to the user, which makes them less 
  insidious than an invisible-by-design tracking feature. "
* Someone proposed that the two URIs would be displayed: Where the user 
  goes and where the ping goes. To which some people answered that it would
  be confusing for the users.

As of now, no decision was made in the thread.

Comment 25

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

To be clear, I don't think people generally felt very strongly about if we should ship it or not. People mostly felt strongly that we don't want to increase user tracking, but I think it has been well established that this does not.

So simply posting to the thread and asking for a conclusion might be a good next step. Personally I don't have strong opinions either way.


On a separate note, please do make sure that you don't repeat some of the mistakes that were made in the initial sendBeacon implementation. I.e. make sure that pings from private-browsing pages use the right cookie jar, and that pings from FirefoxOS apps use the right cookie jar (the latter has still not been fixed which means that sendBeacon is disabled in FirefoxOS).

Comment 26

[(no longer active)]

(In reply to comment #25)
> On a separate note, please do make sure that you don't repeat some of the
> mistakes that were made in the initial sendBeacon implementation. I.e. make
> sure that pings from private-browsing pages use the right cookie jar, and that
> pings from FirefoxOS apps use the right cookie jar (the latter has still not
> been fixed which means that sendBeacon is disabled in FirefoxOS).

Bug 987387 and bug 988107 for your reference.  The former has a patch which shows you how you should respect PB and the latter tells you how the correct cookie jar info should be forwarded to Necko.

Comment 27

[Karl Dubost💡 :karlcow]

To add context to the bug
http://www.whatwg.org/specs/web-apps/current-work/multipage/links.html#hyperlink-auditing
http://www.whatwg.org/specs/web-apps/current-work/multipage/links.html#ping

Comment 28

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Jonas Sicking (:sicking) from comment #23)
> Was there actually any decision in the dev.platform thread?

I did not say that there was a decision. I was merely putting up a patch that fits most of the feedback I have seen.

(In reply to Jonas Sicking (:sicking) from comment #25)
> To be clear, I don't think people generally felt very strongly about if we
> should ship it or not. People mostly felt strongly that we don't want to
> increase user tracking, but I think it has been well established that this
> does not.

Indeed, this was what I understood, too. I would not promote shipping it if it would actually increase user tracking and if it didn't have any value to the user. It does not lead to lesser tracking but does indeed have user value in terms of possible visibility and speed.

> So simply posting to the thread and asking for a conclusion might be a good
> next step. Personally I don't have strong opinions either way.

That's a good idea, will do.

> On a separate note, please do make sure that you don't repeat some of the
> mistakes that were made in the initial sendBeacon implementation. I.e. make
> sure that pings from private-browsing pages use the right cookie jar, and
> that pings from FirefoxOS apps use the right cookie jar (the latter has
> still not been fixed which means that sendBeacon is disabled in FirefoxOS).

I will take a look at this.

Comment 29

[Tim Taubert [:ttaubert] (inactive)]

Additionally to respecting private browsing load contexts, should <a ping> do a CSP check? Should it call CheckLoadURIWithPrincipal() to disallow chrome:, javascript:, etc. URIs? Should it respect CORS?

Does it need to handle SetForceAllowThirdPartyCookie() like the beacon code does? There is a line that references third party blocking already but I don't know how that is related.

I don't know about the last question but I assume the other questions would probably be answered with yes?

Comment 30

[(no longer active)]

(In reply to Tim Taubert [:ttaubert] from comment #29)
> Additionally to respecting private browsing load contexts, should <a ping>
> do a CSP check? Should it call CheckLoadURIWithPrincipal() to disallow
> chrome:, javascript:, etc. URIs? Should it respect CORS?

I don't know, what does the spec say about the above?

> Does it need to handle SetForceAllowThirdPartyCookie() like the beacon code
> does? There is a line that references third party blocking already but I
> don't know how that is related.

I'm not sure what that flag does and when you're supposed to use it.

> I don't know about the last question but I assume the other questions would
> probably be answered with yes?

That would be my guess, but you should also check to see what the spec says we should do there.  :)

Comment 31

[Andrea Giammarchi]

thanks whatwg this is still on hold ... FWIW: here some extra concern that might sound silly but might also cause troubles:

  1. it's too easy to create virtual traffic, all non JS based stats will be misleading (even JS if done wrong)
  2. page loaded at runtime via import or loaders won't benefit from this (pointless in such case)
  3. what a combination of download and ping would do? potentially saturate the SD card without any warning? (I know, silly point ... should be fixed in specs somehow)
  4. what an infinite JS loop generating <a ping> will do? reach bandwidth limits ?
  5. are there ping bandwidth limits at all or we can predownload the internet ?
  4. will a pinged page with a ping link be so smart to pre load that too ?

Comment 32

[Andrea Giammarchi]

thanks whatwg this is still on hold ... FWIW: here some extra concern that might sound silly but might also cause troubles:

  1. it's too easy to create virtual traffic, all non JS based stats will be misleading (even JS if done wrong)
  2. page loaded at runtime via import or loaders won't benefit from this (pointless in such case)
  3. what a combination of download and ping would do? potentially saturate the SD card without any warning? (I know, silly point ... should be fixed in specs somehow)
  4. what an infinite JS loop generating <a ping> will do? reach bandwidth limits ?
  5. are there ping bandwidth limits at all or we can predownload the internet ?
  4. will a pinged page with a ping link be so smart to pre load that too ?

Comment 33

[Andrea Giammarchi]

dunno what happened, probably deleted previous comment. Apologies

Comment 34

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

(In reply to Tim Taubert [:ttaubert] from comment #29)
> Additionally to respecting private browsing load contexts, should <a ping>
> do a CSP check?

Yes. It should be in the same category as XHR.

> Should it call CheckLoadURIWithPrincipal() to disallow
> chrome:, javascript:, etc. URIs?

Yes.

> Should it respect CORS?

Yes.

> Does it need to handle SetForceAllowThirdPartyCookie() like the beacon code
> does?

It should follow the same policy as XHR. If beacon is not then that sounds like a bug.

Comment 35

[Jesse Ruderman]

sendBeacon is now on by default (see bug 990220). Does that make this bug WONTFIX?

Comment 36

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to Jesse Ruderman from comment #35)
> sendBeacon is now on by default (see bug 990220). Does that make this bug
> WONTFIX?

No, I think we want both as they serve somewhat different use cases. Tim, are there any changes we'd need to make our implementation spec compliant? If not we should just go ahead and turn this on.

Comment 37

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #36)
> (In reply to Jesse Ruderman from comment #35)
> > sendBeacon is now on by default (see bug 990220). Does that make this bug
> > WONTFIX?
> 
> No, I think we want both as they serve somewhat different use cases. Tim,
> are there any changes we'd need to make our implementation spec compliant?
> If not we should just go ahead and turn this on.

IIRC there haven't been any changes to the spec and our implementation is still compliant. It hasn't been enabled by default because a lot of people argued that is a subset of sendBeacon(). The only difference is that sendBeacon() is a JS API, <a ping> works without JS.

Comment 38

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Tim Taubert [:ttaubert] from comment #37)
> IIRC there haven't been any changes to the spec and our implementation is
> still compliant. It hasn't been enabled by default because a lot of people
> argued that is a subset of sendBeacon(). The only difference is that
> sendBeacon() is a JS API, <a ping> works without JS.

CSP and the mixed content spec both were written since then, and I think changes are required to conform to those specs.

Comment 39

[Tim Taubert [:ttaubert] (inactive)]

Oh, right. Comment #34 lists a few points.

Comment 40

[Doug Turner (:dougt)]

Comment on attachment 8425338 [details] [diff] [review]
0004-Bug-951104-Enable-a-ping-by-default.patch

see comment #34 and bsmith's concerns.

Comment 41

[Mardeg]

(In reply to Tim Taubert [:ttaubert] from comment #37)
> IIRC there haven't been any changes to the spec and our implementation is
> still compliant.
It looks like there's an intent to change the spec:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Z-RBeFLHRSU

Adding bug 1172217 to the Depends on list.

Comment 42

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I believe what we need to do here is to:

1. Compare our implemented behavior to the spec, and to other implementations (at least Chrome)
2. Fix any differences (not sure if there are any)
3. Make sure that we have tests.
4. Flip the default.

Comment 43

[Anne (:annevk)]

If we have any tests it would also be good to uplift those to web-platform-tests, which appears to have no tests judging from the empty html/semantics/links/downloading-resources/ directory.

Comment 44

[Sylvestre Ledru [:Sylvestre]]

Kyle, do we still care about this bug?

Comment 45

[(no longer active)]

Yes, we do. I believe comment 42 is still accurate.

Comment 46

[Nick Doty]

To help with step 1 of comment 42 and requirements noted in the spec:

• https://bugzilla.mozilla.org/show_bug.cgi?id=401352 notes the unimplemented requirement that users be given visibility that a background auditing side effect will take place;
• https://bugzilla.mozilla.org/show_bug.cgi?id=1546198 notes the unimplemented requirement that users have a control to alter or adjust this behavior

These requirements are noted in
https://www.w3.org/TR/html53/links.html#hyperlink-auditing and
https://html.spec.whatwg.org/multipage/links.html#hyperlink-auditing

With slightly different text in each, both specs provide some suggestions on user controls and user visibility and both note that the potential privacy advantages of <a ping> depend on implementing those requirements. Without implementing these requirements, users risk having less visibility into and control over tracking of their online behavior and sites that don't track this kind of user activity may lose a relative performance advantage.

Comment 47

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Clearing ni? since others answered.

Comment 48

[jscher2000]

ping URLs are already restricted by the user's Tracking Protection setting applicable to the document, which is good and helps meet user expectations. You can confirm using the third link on https://www.jeffersonscher.com/res/pingtest.html in a regular vs. private window.

Is there any thought that aRequireSameHost might be forced to true (overriding browser.send_pings.require_same_host = false) if the user has set custom Content Blocking settings for Cookies to either:

• Third-party trackers (network.cookie.cookieBehavior = 4)
• All third-party cookies (network.cookie.cookieBehavior = 1)

Cookies and pings are not the same thing, but I suspect users would have the same thought process for both.

Comment 49

[Alfredo]

Any update on this?