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

[Proposal] Specify a fragment-directive meant for UA instructions #445

Closed
bokand opened this issue Aug 16, 2019 · 20 comments
Closed

[Proposal] Specify a fragment-directive meant for UA instructions #445

bokand opened this issue Aug 16, 2019 · 20 comments
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest

Comments

@bokand
Copy link

bokand commented Aug 16, 2019

Background

(This use case comes from the ScrollToTextFragment proposal). This is also being discussed as part of the TAG review for this feature in w3ctag/design-reviews#392

The URL fragment is a convenient place to add "UA targeted instructions". For example, a URL with an element-id in the fragment tells the UA load the main resource and scroll the sub-resource indicated by the element-id into view. The media fragments specification adds more ways to address sub-resources. For example, a specific temporal point in a video.

The problem with fragments is that they can be read and processed by script on a page. In some cases, page script will use the fragment to perform its own routing and state tracking. This usage may not interact well with unexpected fragments meant as instructions to the UA. To take a real-world example:

https://www.webmd.com/pain-management/knee-pain/picture-of-the-knee

Navigating to the above URL will load a multi-page article routed using the fragment. If the user tries to load the same URL but with an element-id fragment:

https://www.webmd.com/pain-management/knee-pain/picture-of-the-knee#ContentPane50

The page loads blank because script in the page gets confused.

Proposal

We propose amending the URL specification to allow a ## to indicate a fragment directive. A fragment directive would be a part of the URL reserved for UA instructions that's stripped off during loading before being passed to the page. Taking the example above mixed with our targetText feature:

https://www.webmd.com/pain-management/knee-pain/picture-of-the-knee##targetText=Knee%20Conditions

In this case, the UA processes the targetText and strips it from the URL as seen by the page. From the page's point of view:

window.location.href === 'https://www.webmd.com/pain-management/knee-pain/picture-of-the-knee'
window.location.hash === ''
document.URL === 'https://www.webmd.com/pain-management/knee-pain/picture-of-the-knee'

It can also be combined with plain plain fragments:

https://example.com#routingState-page1##targetText=Header

In which case the page sees:

window.location.href === 'https://example.com#routingState-page1'
window.location.hash === '#routingState-page1'

It also has the benefit of likely being parsed as part of the fragment in URL parsers and won't affect the server request so it should be mostly backwards compatible. The major sticking point is that '#' is currently not a valid code point in URL fragments, something we'd have to change. We're currently trying to determine how compatible this would be; one way is measuring how often we already see URLs with a '#' in the fragment. There's existing discussion in WICG/scroll-to-text-fragment#15.

This would be very helpful in ScrollToTextFragment but we can also imagine it being useful in other new features. For example, as an alternative solution to the html-translate proposal.

Are there any issues we may be overlooking that would make this difficult or undesirable to pursue?

@bokand
Copy link
Author

bokand commented Aug 16, 2019

There are some additional details to consider. I have some initial thoughts on how to handle them as a starting point but this isn't yet fully baked so am open to alternatives:

'hashchange' event

When script does something like:

window.location.hash = 'foo##bar`

We should process the hash and strip off the ##bar part, applying any specified UA instructions. We should only fire 'hashchange' if the location.hash changes. i.e. 'hashchange' is fired only if the part before ## changes.

A counter-argument to this is that the behavior will be different based on whether or not the UA implements ##; however, I think this is a more intuitive behavior.

URL objects

How does new URL() work with ##? I would suggest:

let url = new URL('#foo##bar', 'https://example.com/');
url.toString() === 'https://example.com/#foo##bar'
url.href === 'https://example.com/#foo##bar'
url.hash === '#foo';

When a URL is navigated to, document.URL will have the ## part stripped during loading so that after navigating to the above URL:

// navigated to https://example.com/#foo##bar
url.toString() === 'https://example.com/#foo'
url.href === 'https://example.com/#foo'
url.hash === '#foo';

In other words: the ## part is never visible in hash for URL or Location objects; however, it is visible in the full representation of the URL until it is loaded.

One reason for stripping the fragment directive from the URL on loading is privacy preserving; it helps prevent leaking potentially sensitive information to the page. E.g. the exact text the user is interested in could reveal something sensitive, translation language could be used to help fingerprint. Though, these might be observable in other ways by the page.

A second reason to strip it from href or toString() after loading is to prevent pages from relying on the content inside and thus replicating the existing situation with hash.

Combining base URLs:

let url = new URL('https://example.com#foo')

let directive = new URL('##bar', url);
directive.href === 'https://example.com#foo##bar';

let hash = new URL('#boo', directive)
hash.href === 'https://example.com#boo##bar';

let page = new URL('index.html', directive);
page.href === 'https://example.com/index.html';

In other words: the fragment directive is treated as separate sub-resource identifiers. A changed resource clears the fragment directive.

Anchors

Since the href attribute of an anchor isn't yet loaded:

<a href="https://example.com/#foo##bar">
a.href === 'https://example.com/#foo##bar'

This is consistent with the URL handling above.

Reloads

Should a fragment-directive be reapplied when a page is reloaded? Intuitively, I'd say yes. e.g. If we used the fragment-directive to translate the page, a reload should keep the page in the desired language. This means the UA would need to keep the fragment-directive internally as a reload will occur when the document.URL does not contain the directive.

Multiple directives

If adopted, it's possible in the future there could be several possible uses of the fragment directive (e.g. targetText and htmlTranslate). It should be possible to specify more than one instruction in the fragment directive.

We could adopt the media fragments syntax. Example:

https://example.com##htmlLanguage=es&targetText=esempio

would first translate the page to Spanish, then scroll into view the text snippet "esempio".

Feature detection

Pages can detect if the fragment-directive is supported:

let is_supported = new URL('https://test.com##test').hash === ''

Are there any other aspects, particularly around URL handling, we should be thinking about?

@annevk
Copy link
Member

annevk commented Aug 19, 2019

There should probably be a corresponding issue against the HTML Standard as this impacts navigation. (I'm a bit surprised that setting location.hash to the same value multiple times does not trigger multiple events, this is counter to how we handle "no-op" mutations elsewhere.)

@bokand
Copy link
Author

bokand commented Aug 20, 2019

There should probably be a corresponding issue against the HTML Standard as this impacts navigation.

Do you mean for the entire thing or to extract the loading portion of it? Let me know and I'll follow up on that.

I'm a bit surprised that setting location.hash to the same value multiple times does not trigger multiple events, this is counter to how we handle "no-op" mutations elsewhere.

I'm not sure I understand, setting the same value to hash today doesn't cause multiple events. The proposal as-is wouldn't change that, e.g.:

location.hash = "test##foo"
// hashchange fired
location.hash = "test##bar"
// hashchange *not* fired

Or is the surprise that 'hashchange' isn't fired in the second case? I can see the argument that it may be somewhat surprising but reading location.hash will return "#test" in both cases so I think not firing the event is more intuitive.

@annevk
Copy link
Member

annevk commented Aug 21, 2019

In particular how it affects navigation and scrolling (both defined to some extent in HTML).

(The surprise is that hashchange isn't fired twice when you set location.hash to the same value twice, as mutation observers would run twice for an equivalent scenario for instance. It wasn't really a comment on the proposal.)

@bokand
Copy link
Author

bokand commented Aug 28, 2019

Thanks, I've opened whatwg/html#4868

One additional thing to point out: the ## delimiter is somewhat arbitrary. It's nice because it means we can specify it alone in a URL (e.g. example.com##directive). The downside, as noted above, is that it isn't valid today. Does anyone have a sense of how brittle existing tools would be to making this change? I've tried the major browsers and they all seem to cope. Are there popular URL parser libraries I should try out?

As an alternative, we could find (via browser metrics, HTTPArchive) a alternative delimiter that's valid but rarely used. E.g. https://example.com#fragment::targetText=foo

@masinter
Copy link

masinter commented Sep 5, 2019

in IETF specifications, the fragment is not transmitted as part of the HTTP request or any other kind of resolution/fetch. The fragment syntax and interpretation is determined entirely by the MIME type of the fetched result. It would be confusing and unfortunate to change that.
Rather, restrict this interpretation to text/html.

@bokand
Copy link
Author

bokand commented Sep 5, 2019

Right, I don't think we'd want to change that. The interpretation of the fragment into a directive would be left to HTML document loading. I think the only change we'd need in the URL spec would be to allow a '#' character in fragments as today this would be considered an invalid URL. e.g. example.org##foo is invalid. Otherwise I think a URL like that should behave as a fragment does today.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 6, 2019
Scroll to text defines a double-hash as the URL fragment directive[1].
The fragment directive should always be stripped from the URL to avoid
breaking pages that use the fragment for state. Our implementation
previously only stripped the fragment directive if we parsed targetText.

Also improved the web platform test to test whether the target scrolls
to the element or text fragment as expected.

Tested updated WPT locally with
run_web_tests.py --additional-driver-flag=
  '--enable-blink-features=TextFragmentIdentifiers'

[1] whatwg/url#445

Bug: 994818
Change-Id: I48109683a5e5ba162f1db72b1c5b174f3b017251
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 6, 2019
Scroll to text defines a double-hash as the URL fragment directive[1].
The fragment directive should always be stripped from the URL to avoid
breaking pages that use the fragment for state. Our implementation
previously only stripped the fragment directive if we parsed targetText.

Also improved the web platform test to test whether the target scrolls
to the element or text fragment as expected.

Tested updated WPT locally with
run_web_tests.py --additional-driver-flag=
  '--enable-blink-features=TextFragmentIdentifiers'

[1] whatwg/url#445

Bug: 994818
Change-Id: I48109683a5e5ba162f1db72b1c5b174f3b017251
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 6, 2019
Scroll to text defines a double-hash as the URL fragment directive[1].
The fragment directive should always be stripped from the URL to avoid
breaking pages that use the fragment for state. Our implementation
previously only stripped the fragment directive if we parsed targetText.

Also improved the web platform test to test whether the target scrolls
to the element or text fragment as expected.

Tested updated WPT locally with
run_web_tests.py --additional-driver-flag=
  '--enable-blink-features=TextFragmentIdentifiers'

[1] whatwg/url#445

Bug: 994818
Change-Id: I48109683a5e5ba162f1db72b1c5b174f3b017251
aarongable pushed a commit to chromium/chromium that referenced this issue Sep 6, 2019
Scroll to text defines a double-hash as the URL fragment directive[1].
The fragment directive should always be stripped from the URL to avoid
breaking pages that use the fragment for state. Our implementation
previously only stripped the fragment directive if we parsed targetText.

Also improved the web platform test to test whether the target scrolls
to the element or text fragment as expected.

Tested updated WPT locally with
run_web_tests.py --additional-driver-flag=
  '--enable-blink-features=TextFragmentIdentifiers'

[1] whatwg/url#445

Bug: 994818
Change-Id: I48109683a5e5ba162f1db72b1c5b174f3b017251
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1772166
Commit-Queue: Nick Burris <nburris@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#694407}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 6, 2019
Scroll to text defines a double-hash as the URL fragment directive[1].
The fragment directive should always be stripped from the URL to avoid
breaking pages that use the fragment for state. Our implementation
previously only stripped the fragment directive if we parsed targetText.

Also improved the web platform test to test whether the target scrolls
to the element or text fragment as expected.

Tested updated WPT locally with
run_web_tests.py --additional-driver-flag=
  '--enable-blink-features=TextFragmentIdentifiers'

[1] whatwg/url#445

Bug: 994818
Change-Id: I48109683a5e5ba162f1db72b1c5b174f3b017251
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1772166
Commit-Queue: Nick Burris <nburris@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#694407}
Hexcles pushed a commit to web-platform-tests/wpt that referenced this issue Sep 9, 2019
Scroll to text defines a double-hash as the URL fragment directive[1].
The fragment directive should always be stripped from the URL to avoid
breaking pages that use the fragment for state. Our implementation
previously only stripped the fragment directive if we parsed targetText.

Also improved the web platform test to test whether the target scrolls
to the element or text fragment as expected.

Tested updated WPT locally with
run_web_tests.py --additional-driver-flag=
  '--enable-blink-features=TextFragmentIdentifiers'

[1] whatwg/url#445

Bug: 994818
Change-Id: I48109683a5e5ba162f1db72b1c5b174f3b017251
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1772166
Commit-Queue: Nick Burris <nburris@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#694407}
LukeZielinski pushed a commit to web-platform-tests/wpt that referenced this issue Sep 9, 2019
Scroll to text defines a double-hash as the URL fragment directive[1].
The fragment directive should always be stripped from the URL to avoid
breaking pages that use the fragment for state. Our implementation
previously only stripped the fragment directive if we parsed targetText.

Also improved the web platform test to test whether the target scrolls
to the element or text fragment as expected.

Tested updated WPT locally with
run_web_tests.py --additional-driver-flag=
  '--enable-blink-features=TextFragmentIdentifiers'

[1] whatwg/url#445

Bug: 994818
Change-Id: I48109683a5e5ba162f1db72b1c5b174f3b017251
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1772166
Commit-Queue: Nick Burris <nburris@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#694407}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Sep 14, 2019
…scroll-to-text WPT, a=testonly

Automatic update from web-platform-tests
Strip the fragment directive and update scroll-to-text WPT

Scroll to text defines a double-hash as the URL fragment directive[1].
The fragment directive should always be stripped from the URL to avoid
breaking pages that use the fragment for state. Our implementation
previously only stripped the fragment directive if we parsed targetText.

Also improved the web platform test to test whether the target scrolls
to the element or text fragment as expected.

Tested updated WPT locally with
run_web_tests.py --additional-driver-flag=
  '--enable-blink-features=TextFragmentIdentifiers'

[1] whatwg/url#445

Bug: 994818
Change-Id: I48109683a5e5ba162f1db72b1c5b174f3b017251
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1772166
Commit-Queue: Nick Burris <nburris@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#694407}

--

wpt-commits: 603a271948a7162bc6efc3c882856e618eabb30e
wpt-pr: 18898
xeonchen pushed a commit to xeonchen/gecko that referenced this issue Sep 14, 2019
…scroll-to-text WPT, a=testonly

Automatic update from web-platform-tests
Strip the fragment directive and update scroll-to-text WPT

Scroll to text defines a double-hash as the URL fragment directive[1].
The fragment directive should always be stripped from the URL to avoid
breaking pages that use the fragment for state. Our implementation
previously only stripped the fragment directive if we parsed targetText.

Also improved the web platform test to test whether the target scrolls
to the element or text fragment as expected.

Tested updated WPT locally with
run_web_tests.py --additional-driver-flag=
  '--enable-blink-features=TextFragmentIdentifiers'

[1] whatwg/url#445

Bug: 994818
Change-Id: I48109683a5e5ba162f1db72b1c5b174f3b017251
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1772166
Commit-Queue: Nick Burris <nburris@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#694407}

--

wpt-commits: 603a271948a7162bc6efc3c882856e618eabb30e
wpt-pr: 18898
@bokand
Copy link
Author

bokand commented Oct 4, 2019

Given the point raised in the URI mailing list we've since changed to use existing fragment code-points for the fragment directive.

We still have the concept of the fragment directive but it's now delimited by :~: rather than ## and the processing is done entirely in the HTML spec during fragment processing steps. I don't think there's any changes required in the URL spec. We're still interested in feedback but given there's no URL changes I'm going to close this issue, feel free to file issues in WICG/ScrollToTextFragment or in whatwg/html#4868.

@bokand bokand closed this as completed Oct 4, 2019
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 4, 2019
…scroll-to-text WPT, a=testonly

Automatic update from web-platform-tests
Strip the fragment directive and update scroll-to-text WPT

Scroll to text defines a double-hash as the URL fragment directive[1].
The fragment directive should always be stripped from the URL to avoid
breaking pages that use the fragment for state. Our implementation
previously only stripped the fragment directive if we parsed targetText.

Also improved the web platform test to test whether the target scrolls
to the element or text fragment as expected.

Tested updated WPT locally with
run_web_tests.py --additional-driver-flag=
  '--enable-blink-features=TextFragmentIdentifiers'

[1] whatwg/url#445

Bug: 994818
Change-Id: I48109683a5e5ba162f1db72b1c5b174f3b017251
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1772166
Commit-Queue: Nick Burris <nburrischromium.org>
Reviewed-by: David Bokan <bokanchromium.org>
Cr-Commit-Position: refs/heads/master{#694407}

--

wpt-commits: 603a271948a7162bc6efc3c882856e618eabb30e
wpt-pr: 18898

UltraBlame original commit: d6a259fed875176e1f7bc83e32e1eb6cc9ef1fb0
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 4, 2019
…scroll-to-text WPT, a=testonly

Automatic update from web-platform-tests
Strip the fragment directive and update scroll-to-text WPT

Scroll to text defines a double-hash as the URL fragment directive[1].
The fragment directive should always be stripped from the URL to avoid
breaking pages that use the fragment for state. Our implementation
previously only stripped the fragment directive if we parsed targetText.

Also improved the web platform test to test whether the target scrolls
to the element or text fragment as expected.

Tested updated WPT locally with
run_web_tests.py --additional-driver-flag=
  '--enable-blink-features=TextFragmentIdentifiers'

[1] whatwg/url#445

Bug: 994818
Change-Id: I48109683a5e5ba162f1db72b1c5b174f3b017251
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1772166
Commit-Queue: Nick Burris <nburrischromium.org>
Reviewed-by: David Bokan <bokanchromium.org>
Cr-Commit-Position: refs/heads/master{#694407}

--

wpt-commits: 603a271948a7162bc6efc3c882856e618eabb30e
wpt-pr: 18898

UltraBlame original commit: d6a259fed875176e1f7bc83e32e1eb6cc9ef1fb0
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 4, 2019
…scroll-to-text WPT, a=testonly

Automatic update from web-platform-tests
Strip the fragment directive and update scroll-to-text WPT

Scroll to text defines a double-hash as the URL fragment directive[1].
The fragment directive should always be stripped from the URL to avoid
breaking pages that use the fragment for state. Our implementation
previously only stripped the fragment directive if we parsed targetText.

Also improved the web platform test to test whether the target scrolls
to the element or text fragment as expected.

Tested updated WPT locally with
run_web_tests.py --additional-driver-flag=
  '--enable-blink-features=TextFragmentIdentifiers'

[1] whatwg/url#445

Bug: 994818
Change-Id: I48109683a5e5ba162f1db72b1c5b174f3b017251
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1772166
Commit-Queue: Nick Burris <nburrischromium.org>
Reviewed-by: David Bokan <bokanchromium.org>
Cr-Commit-Position: refs/heads/master{#694407}

--

wpt-commits: 603a271948a7162bc6efc3c882856e618eabb30e
wpt-pr: 18898

UltraBlame original commit: d6a259fed875176e1f7bc83e32e1eb6cc9ef1fb0
@bokand
Copy link
Author

bokand commented Oct 10, 2019

I spoke too soon. There are still some changes that we'd need to merge into the URL spec, they're just much less scary. The additions are to parsing and serializing to separate the fragment-directive from the fragment: https://wicg.github.io/ScrollToTextFragment/#parsing-the-fragment-directive

@bokand bokand reopened this Oct 10, 2019
@annevk
Copy link
Member

annevk commented Oct 10, 2019

Couldn't this be layered entirely on top of a URL record and not require changes to the URL parser?

It would help if you could explain the high-level idea as to why the URL Standard is impacted here as it's not immediately clear if that document reflects the current state of affairs.

@bokand
Copy link
Author

bokand commented Oct 10, 2019

Hmm, perhaps. The idea is that we want to separate the URL fragment into two pieces: the "legacy fragment" and the "directive". e.g.: https://example.com#fragment:~:directive

Where "fragment" is placed in the URL's fragment field and "directive" is placed in the URL's fragment-directive field.

That said, we could keep the entire thing in the [fragment field] of the URL and perform all the same steps in the HTML spec. It'd require mutating the fragment field during loading and defining it as part of URL parsing seemed cleaner. However, I can appreciate that changing URL spec seems like a bigger deal (it's more foundational) so if you think keeping it all in HTML is a better approach I'd be happy to close this in favour of it.

@bokand
Copy link
Author

bokand commented Oct 10, 2019

And to provide some high level motivation, the idea is that we want to provide instructions to the UA in the directive since that won't be exposed to page script. This allows a feature like ScrollToTextFragment to work on pages that use the fragment from script for routing and state

@annevk
Copy link
Member

annevk commented Oct 10, 2019

I see, but at that point you are still changing how a URL is parsed, which I thought folks successfully argued against doing.

It definitely seems preferable to keep fragment the way it is today, i.e., fragment:~:directive, and perhaps offer a special API that returns fragment / directive on top. Some of that infrastructure could be maybe be in the URL Standard, similar to how it supports blob URLs, but changing the fundamental model seems more problematic.

@bokand
Copy link
Author

bokand commented Oct 10, 2019

I see, but at that point you are still changing how a URL is parsed, which I thought folks successfully argued against doing.

The main concern was using the # code point inside a fragment since that could trip up existing parsers.

It definitely seems preferable to keep fragment the way it is today

Just want to make sure I understand you: by "preferable" do you mean as opposed to splitting fragment and directive during parsing the URL spec? Or as opposed to doing that in the HTML spec as well?

At some point in the process we need to remove the directive from the fragment. In our current implementation we actually do this during Document loading. We create a new URL based on the original URL but with the directive stripped out and set that on the Document - come to think of it, maybe that's a better way to specify it. WDYT?

@annevk
Copy link
Member

annevk commented Oct 10, 2019

I don't know, what if you set location.hash to something that contains this? Some kind of split can be made during navigation, but it's not clear to me it should affect document.URL or some such (or the URL parser for that matter).

@bokand
Copy link
Author

bokand commented Oct 10, 2019

The text directive isn't invoked on same-document navigations so I think the fact it wouldn't affect setting location.hash would work (at least for our case of text fragments, maybe it's limiting for future use cases?).

We intentionally want to hide the specified text from page script; even the destination origin shouldn't be able to tell what you've come searching for as that could leak privacy sensitive information. So I think affecting document.URL (by removing the directive from it) is desirable.

There's some compat risk to this but we've done quite a bit of investigation here, both with Chrome telemetry and scraping the Google search crawler, and feel reasonably confident that this won't affect any sites in the wild (hence the odd-looking choice of :~:).

pdigennaro pushed a commit to washezium/washezium that referenced this issue Nov 4, 2019
Scroll to text defines a double-hash as the URL fragment directive[1].
The fragment directive should always be stripped from the URL to avoid
breaking pages that use the fragment for state. Our implementation
previously only stripped the fragment directive if we parsed targetText.

Also improved the web platform test to test whether the target scrolls
to the element or text fragment as expected.

Tested updated WPT locally with
run_web_tests.py --additional-driver-flag=
  '--enable-blink-features=TextFragmentIdentifiers'

[1] whatwg/url#445

(cherry picked from commit 9d7721d)

Bug: 994818
Change-Id: I48109683a5e5ba162f1db72b1c5b174f3b017251
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1772166
Commit-Queue: Nick Burris <nburris@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#694407}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1829307
Reviewed-by: Nick Burris <nburris@chromium.org>
Cr-Commit-Position: refs/branch-heads/3904@{#516}
Cr-Branched-From: 675968a-refs/heads/master@{#693954}
@annevk annevk added addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest labels May 5, 2020
@annevk
Copy link
Member

annevk commented May 10, 2020

What's the proposal for the URL Standard at this point?

@bokand
Copy link
Author

bokand commented May 12, 2020

I don't think we want to change anything fundamental about URLs. It's possible when it comes to moving things into specs, as you say, some of the infrastructure could be put here but for now I think we can close this.

@bokand bokand closed this as completed May 12, 2020
@annevk
Copy link
Member

annevk commented May 12, 2020

@bokand should #389 be closed as well?

@bokand
Copy link
Author

bokand commented May 15, 2020

I think so, replied there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest
Development

No branches or pull requests

3 participants