autofocus: Loosen fragment identifier restriction

[tkent-google]

Fixes #5252

Before this change:
foo.html# => autofocus works
foo.html#top => autofocus does not work
foo.html#existingId => autofocus does not work
foo.html#non-existent => autofocus does not work

After this change:
foo.html# => autofocus works
foo.html#top => autofocus works
foo.html#existingId => autofocus does not work
foo.html#non-existent => autofocus works

• At least two implementers are interested (and none opposed):
  • Chrome: I'll implement this.
  • Firefox: Probably HTML: autofocus="" overhaul mozilla/standards-positions#195 is enough because this is a small fix in the new autofocus algorithm, which Firefox doesn't implement yet.
• Tests are written and can be reviewed and commented upon at:
  • html: Update autofocus tests in cases of documents with fragments web-platform-tests/wpt#26815
• Implementation bugs are filed:
  • Chrome: http://crbug.com/1046357
  • Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=1574435
  • Safari: https://bugs.webkit.org/show_bug.cgi?id=203139

(See WHATWG Working Mode: Changes for more details.)

---

/interaction.html ( diff )

[domenic]

The worry I have here is exposing the presence or absence of a target element for cross-origin ancestor documents. That seems not great security-wise. Also, it is probably hard to implement, in out-of-process-iframe cases?

[tkent-google]

The worry I have here is exposing the presence or absence of a target element for cross-origin ancestor documents. That seems not great security-wise. Also, it is probably hard to implement, in out-of-process-iframe cases?

I thought we avoided such situations by "If target's origin is not the same as the origin of topDocument, then return." in the autofocus element insertion steps, however I found it didn't work well if an iframe in the middle had a different origin.

Top frame: origin-1
Child iframe: origin-2
Grandchild iframe: origin-1

I updated the step to solve this issue.

[domenic]

LGTM. @annevk, could you double-check, since I know ancestor-crawling is always a bit fragile? Note that "flush autofocus candidates" is only ever run when the document is fully active, so I think the current text is good.

Also, I tend to agree with @tkent-google that probably this minor tweak (almost a bug fix) is covered by Mozilla's previous endorsement of the autofocus revamp, but your take on that would be appreciated as well.

[tkent-google]

Top frame: origin-1
Child iframe: origin-2
Grandchild iframe: origin-1

Another solution would be to drop different origin documents in the inclusiveAncestorDocuments collection step. What do you think?

[annevk]

I'm not sure I understand the security setup. Couldn't a same-origin document (with respect to the top-level document) always steal focus, regardless of documents in between? Or would this not work if one of the documents in between had focus? (In which case the check should probably be relative to who has focus.)

[domenic]

Another solution would be to drop different origin documents in the inclusiveAncestorDocuments collection step. What do you think?

I slightly prefer your current setup, because it seems more obviously stopping the bad thing at the source. But I don't have a strong preference.

I'm not sure I understand the security setup. Couldn't a same-origin document (with respect to the top-level document) always steal focus, regardless of documents in between? Or would this not work if one of the documents in between had focus? (In which case the check should probably be relative to who has focus.)

The attack scenario I'm concerned about is a small cross-origin information leak, not about focus stealing. In particular, consider A1 -> B -> A2 where B can be in two states:

1. B has a target element, e.g., B is currently navigated to https://b.example.com/#foo and an element with ID foo exists

2. B has no target element, e.g., B is currently navigated to https://b.example.com/#foo and no element with ID foo exists

Now, A2 contains <input autofocus>. Without the step introduced in ea99b99 , A2 can tell:

• If the input is focused, then B is in state 2.
• In the input remains unfocused, then B is in state 1.

Does that make sense?

[annevk]

The longer I look at this algorithm the more confused I get. We only run "flush autofocus candidates" for a top-level document. Why would it ever have ancestors?

If that's a misunderstanding somehow I don't understand why it cares about the fragment of ancestor documents and not whether or not they are focused. If a document has a fragment and then the user scrolls around, it seems the fragment is no longer relevant even if it's still there?

[tkent-google]

We only run "flush autofocus candidates" for a top-level document. Why would it ever have ancestors?

The candidates can contain elements in descendant documents.

If that's a misunderstanding somehow I don't understand why it cares about the fragment of ancestor documents and not whether or not they are focused. If a document has a fragment and then the user scrolls around, it seems the fragment is no longer relevant even if it's still there?

A user or a code expects that a document is scrolled to a specific position by a URL fragment. Autofocus should not break the expectation. If autofocus worked after scroll-to-fragment, the document would be scrolled unexpectedly without any user activation.

[annevk]

Thanks! But doesn't it mean that if we have a single document whose fragment is not empty, that upon some user action inserts a new widget that contains autofocus, it would not get focus? But focus() would? (This might make more sense if "autofocus processed flag" was always set once a document was "loaded", but that doesn't seem to be the case.)

I also come back to the question of who has focus. In Domenic's scenario, assuming A1 used that URL with fragment for B, B doesn't have focus afaik, so it seems fine for A2 to take focus from A1, regardless of how B is scrolled.

Now in the setup where A1 loads B without fragment and B loads A2, and then the user navigates B to a fragment, it doesn't make sense for A2 to steal focus, but mostly because that would be across origins. Setting the "autofocus processed flag" at some point during loading would also help with that.

[smaug----]

Isn't this quite racy if target element happens to be somewhere close to the end of the document?

[tkent-google]

You're right. the order of scroll-to-fragment and the autofocus processing is not deterministic. But regardless of their order, the final focused element will be the :target element.

[tkent-google]

Thanks! But doesn't it mean that if we have a single document whose fragment is not empty, that upon some user action inserts a new widget that contains autofocus, it would not get focus? But focus() would? (This might make more sense if "autofocus processed flag" was always set once a document was "loaded", but that doesn't seem to be the case.)

Yes, it does. Note that this PR changes "fragment is not empty" to ":target exists".
As for the difference between autofocus and focus(), I think autofocus is basically a feature for the loading time. Setting "autofocus processed flag" on finishing document load makes sense, but I'm afraid it will break too many sites.

I also come back to the question of who has focus. In Domenic's scenario, assuming A1 used that URL with fragment for B, B doesn't have focus afaik, so it seems fine for A2 to take focus from A1, regardless of how B is scrolled.

Yes. If the fragment is provided by the code in A1, not by the user, I think it's ok that autofocus in A2 works.

Now in the setup where A1 loads B without fragment and B loads A2, and then the user navigates B to a fragment, it doesn't make sense for A2 to steal focus, but mostly because that would be across origins. Setting the "autofocus processed flag" at some point during loading would also help with that.

Yeah, it looks to work well.

Summary of my opinion:

• I think setting "autofocus processed flag" on finishing document load is risky for site compatibility.
• Nested frame like A1 -> B -> A2 must be rare. I guess disabling autofocus in A2 is safer than setting "autofocus processed flag" on finishing document load.

[annevk]

The other question I had is why it needs to be disabled in this way and why the origin check cannot be with the origin of the document that currently has focus.

[tkent-google]

The other question I had is why it needs to be disabled in this way

Do you have other ideas?

why the origin check cannot be with the origin of the document that currently has focus.

It would not make much sense because if a descendant document has focus, 'autofocus' should not work at all.

[annevk]

I see, I guess that means you have to go through ancestors and then I don't really have other ideas.

[domenic]

LGTM. I'll give folks a day or so to comment and then merge this.