Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report URL in "name" attribute (instead of "document") #59

Closed
spanicker opened this issue Dec 19, 2016 · 14 comments
Closed

Report URL in "name" attribute (instead of "document") #59

spanicker opened this issue Dec 19, 2016 · 14 comments
Milestone

Comments

@spanicker
Copy link

NT2 "name" attribute is currently set to "document".
Should this be set to the URL instead (as in RT2) ?
Related discussion in: https://crbug.com/675039

@igrigorik
Copy link
Member

@toddreifsteck if we wanted to align RT and NT in this regard, and report the URL in both.. is that something Edge would be able to align with?

@toddreifsteck
Copy link
Member

toddreifsteck commented Jan 18, 2017

We should reach out to the various Analytics vendors who are part of the group as well as internal Analytics folks at our various companies.

IE11 and Edge 12-15 currently have shipped with this name='document' behavior for Navigation Timing 2. Although I have no pushback on the value of this update, I have concerns about breaking this behavior given years of a shipping implementation. If we choose to make this change, we should be clear that we are potentially breaking existing web code and should have some due diligence.

@spanicker Perhaps Chrome can ship with name==URL and add Telemetry for getEntriesByName('document') and access to the entry.name entry on NavigationTiming2 objects?

@igrigorik
Copy link
Member

We should reach out to the various Analytics vendors who are part of the group as well as internal Analytics folks at our various companies.

/cc @nicjansma

@nicjansma
Copy link

I agree that it makes more sense to have name be the document's URL instead of "document" for consistency with RT, though, most people will get the URL from window.location anyways.

Note boomerang does not query for NT2 in the Perf Timeline -- we query performance.timing (which doesn't have the name attribute).

I haven't seen anyone query for getEntriesByName("document") (other than W3C tests) to get NT from the PerformanceTimeline -- and there's only a few examples for getEntriesByType("navigation") (according to Google searches). (My guess is most people just use performance.timing which has been around longer and provides the same data in a different epoch)

Telemetry to validate our assumptions is always good :)

@spanicker
Copy link
Author

Unless web devs / Nic indicate that moving to URL improves usability significantly, I'm not sure it's worth the additional last minute drill for NT2, given that it'll take some time & effort to investigate the compatibility issue.

@nicjansma
Copy link

I think it would be a requirement for fixing #37, since in that case you would want to know the actual URL of each redirect.

@spanicker
Copy link
Author

@igrigorik if you want this for M57, please file a crbug and I'll try to get it pulled in to the release.

@igrigorik
Copy link
Member

@nicjansma yep, exactly.

@spanicker we already have https://bugs.chromium.org/p/chromium/issues/detail?id=675039, do we need another one? Personally, I'd strongly prefer we make this change now, before we have to implementations in the wild..

@cvazac
Copy link

cvazac commented Jan 20, 2017

I haven't yet seen anyone rely on name === 'document'. (I checked episodes and newrelic.)

If this change is made, I suggest the value of name be the requested URL as opposed to the redirect URL. Redirect entries might look like:

PerformanceNavigationTiming
 entryType: navigation
 name: http://www.mysite.com <!-- requested url -->
 redirect_url: https://www.mysite.com

PerformanceNavigationTiming
 entryType: redirect
 name: https://www.mysite.com

jeffcarp pushed a commit to web-platform-tests/wpt that referenced this issue Feb 7, 2017
There's a strong request to merge this in M57, see spec bug: w3c/navigation-timing#59

BUG=675039

Review-Url: https://codereview.chromium.org/2675973004
Cr-Commit-Position: refs/heads/master@{#448517}
jeffcarp added a commit to web-platform-tests/wpt that referenced this issue Feb 7, 2017
Update NT2 name to be the requested URL instead of 'document' There's a strong request to merge this in M57, see spec bug: w3c/navigation-timing#59
MXEBot pushed a commit to mirror/chromium that referenced this issue Feb 7, 2017
There's a strong request to merge this in M57, see spec bug: w3c/navigation-timing#59

BUG=675039

Review-Url: https://codereview.chromium.org/2675973004
Cr-Commit-Position: refs/heads/master@{#448517}
MXEBot pushed a commit to mirror/chromium that referenced this issue Feb 9, 2017
… a strong request to merge this in M57, see spec bug: w3c/navigation-timing#59

BUG=675039

Review-Url: https://codereview.chromium.org/2675973004
Cr-Commit-Position: refs/heads/master@{#448517}
(cherry picked from commit 0af62af)

Review-Url: https://codereview.chromium.org/2686973002 .
Cr-Commit-Position: refs/branch-heads/2987@{#397}
Cr-Branched-From: ad51088-refs/heads/master@{#444943}
@igrigorik
Copy link
Member

If this change is made, I suggest the value of name be the requested URL as opposed to the redirect URL.

The other way around, we need to report the final URL (window.location.toString()), otherwise we'll be reporting wrong timestamps for the advertised URL in the future; we can't expose the original URL unless that response passes TAO opt-in.

igrigorik added a commit that referenced this issue Feb 24, 2017
@igrigorik igrigorik added this to the Level 2 milestone Feb 24, 2017
igrigorik added a commit that referenced this issue Feb 24, 2017
@igrigorik
Copy link
Member

  • shipping in M57 in Chrome, stable ~mid march
  • we'll pause landing it until we have confidence in it

Check back in 2~3 weeks.

@foolip
Copy link
Member

foolip commented Mar 7, 2017

I came here from https://lists.w3.org/Archives/Public/public-test-infra/2017JanMar/0025.html

Is it a correct reading of this issue that everyone can live with the proposed behavior, but want to see it proven web compatible before actually committing to the change? This is a fairly common situation, but does make things tricky with Chromium's new 2-way wpt sync. Will comment further on public-test-infra.

@toddreifsteck
Copy link
Member

@foolip That is the exact concern.

@patrickkettner flagged so he is aware of this thread.

igrigorik added a commit that referenced this issue Jun 29, 2017
@plehegar
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants