Tim Taubert [:ttaubert] (inactive) Reporter
	Description • 11 years ago

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.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 1 • 11 years ago

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.

	Tim Taubert [:ttaubert] (inactive) Reporter
	Comment 2 • 11 years ago

(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.

	Mark Finkle (:mfinkle) (use needinfo?)
	Comment 3 • 11 years ago

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

	Doug Turner (:dougt)
	Comment 4 • 11 years ago

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.

	Andrea Giammarchi
	Comment 5 • 11 years ago

> 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

	Mark Finkle (:mfinkle) (use needinfo?)
	Comment 6 • 11 years ago

(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.

	Mark Finkle (:mfinkle) (use needinfo?)
	Comment 7 • 11 years ago

(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?

	Doug Turner (:dougt)
	Comment 8 • 11 years ago

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

	Doug Turner (:dougt)
	Comment 9 • 11 years ago

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

	Andrea Giammarchi
	Comment 10 • 11 years ago

(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 ...

	Mark Finkle (:mfinkle) (use needinfo?)
	Comment 11 • 11 years ago

(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

	Doug Turner (:dougt)
	Comment 12 • 11 years ago

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

	Andrea Giammarchi
	Comment 13 • 11 years ago

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

	Jonas Sicking (:sicking) No longer reading bugmail consistently
	Comment 14 • 11 years ago

> 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.

	Jonas Sicking (:sicking) No longer reading bugmail consistently
	Comment 15 • 11 years ago

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.

	Brian Smith (:briansmith, :bsmith, use NEEDINFO?)
	Comment 16 • 11 years ago

(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.

	Jonas Sicking (:sicking) No longer reading bugmail consistently
	Comment 17 • 11 years ago

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.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 18 • 11 years ago

(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.

	Doug Turner (:dougt)
	Comment 19 • 11 years ago

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.

	Karl Dubost💡 :karlcow
	Comment 20 • 11 years ago

*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.

	Doug Turner (:dougt)
	Comment 21 • 10 years ago

I am okay if we ignore me in comment 19.

	Tim Taubert [:ttaubert] (inactive) Reporter
	Comment 22 • 10 years ago

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.

	Jonas Sicking (:sicking) No longer reading bugmail consistently
	Comment 23 • 10 years ago

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

	Karl Dubost💡 :karlcow
	Comment 24 • 10 years ago

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.

	Jonas Sicking (:sicking) No longer reading bugmail consistently
	Comment 25 • 10 years ago

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).

	(no longer active)
	Comment 26 • 10 years ago

(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.

	Karl Dubost💡 :karlcow
	Comment 27 • 10 years ago

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

	Tim Taubert [:ttaubert] (inactive) Reporter
	Comment 28 • 10 years ago

(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.

	Tim Taubert [:ttaubert] (inactive) Reporter
	Comment 29 • 10 years ago

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?

	(no longer active)
	Comment 30 • 10 years ago

(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.  :)

	Andrea Giammarchi
	Comment 31 • 10 years ago

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 ?

	Jonas Sicking (:sicking) No longer reading bugmail consistently
	Comment 34 • 10 years ago

(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.

	Jesse Ruderman
	Comment 35 • 10 years ago

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

	Brad Lassey [:blassey] (use needinfo?)
	Comment 36 • 10 years ago

(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.

	Tim Taubert [:ttaubert] (inactive) Reporter
	Comment 37 • 10 years ago

(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.

	Brian Smith (:briansmith, :bsmith, use NEEDINFO?)
	Comment 38 • 10 years ago

(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.

	Tim Taubert [:ttaubert] (inactive) Reporter
	Comment 39 • 10 years ago

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

	Doug Turner (:dougt)
	Comment 40 • 10 years ago

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

see comment #34 and bsmith's concerns.

	Mardeg
	Comment 41 • 9 years ago

(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.

	Jonas Sicking (:sicking) No longer reading bugmail consistently
	Comment 42 • 8 years ago

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.

	Anne (:annevk)
	Comment 43 • 7 years ago

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.