search

WICG / scroll-to-text-fragment

Proposal to allow specifying a text snippet in a URL fragment
Other
586 stars 43 forks source link

Don't set focus to text fragments across origins #227

Open zcorpan opened 1 year ago

zcorpan commented 1 year ago

https://wicg.github.io/scroll-to-text-fragment/#issue-e253a983 says

Run the focusing steps for target, with the Document’s viewport as the fallback target. (Issue 5) Implementation note: Blink doesn’t currently set focus for text fragments, it probably should? TODO: file crbug.

and https://web.dev/text-fragments/#privacy says

Since a naive Text Fragments implementation might decide that a successful match should cause a focus switch, on evil-ads.example.com I could listen for the blur event and thus know when a match occurred. In Chrome, we have implemented Text Fragments in such a way that the above scenario cannot happen.

This suggests the spec here is wrong and focusing target, at least across origins, would be a privacy leak.

bokand commented 1 year ago

I think the real mitigation here is that scrolling won't occur if the initiator of the navigation isn't same-origin:

If the navigationParam’s request has a sec-fetch-site header and its value is "same-origin" set allow text fragment scroll to true and abort these sub-steps.

However, I don't feel too strongly about this since I'm not sure applying focus is super useful anyway (we do set sequential focus for keyboard navigation and accessibility, which are very useful, but I think those aren't programmatically detectable?).

annevk commented 1 year ago

"Focusing steps" takes an element or navigable, so it seems to me there's a logic error here.

bokand commented 1 year ago

The spec adds a monkey patch to make target the lowest-common-ancestor Element when scrolling to a text-fragment Range.

bokand commented 8 months ago

Hmm, taking a closer look - it seems Chrome doesn't actually apply focus, nor does Safari. I can't think of how applying focus would be useful here and given the potential for leaks it adds I'd err to avoiding the focus steps (for text directives) in the spec. Any objections?

zcorpan commented 4 months ago

@jnjaeschke what do you think?