document.querySelector(':target') fails to select :target element before page load

UNCONFIRMED
Unassigned

Status

()

Core
DOM: Core & HTML
UNCONFIRMED
2 years ago
2 years ago

People

(Reporter: zzzzBov, Unassigned)

Tracking

41 Branch
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

2 years ago
User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2490.80 Safari/537.36

Steps to reproduce:

1. visit a url that includes a document fragment that points to a DOM element
2. call document.querySelector(':target') or document.querySelectorAll(':target') after the element was created, but before the page has loaded:

e.x. example.html#test

<div id="test"></div>
<script>
  var targetTest = document.querySelector(':target'); //null
  var hashTest = document.querySelector(location.hash); //<div id="test"></div>
</script>


Actual results:

The return value of document.querySelector will be null, and querySelectorAll will return an empty collection.


Expected results:

The return value of document.querySelector should be the DOM node that would be selected by document.querySelector(location.hash)
(Reporter)

Comment 1

2 years ago
Relates to https://github.com/jquery/jquery/issues/2679

Updated

2 years ago
Component: Untriaged → DOM: Core & HTML
Product: Firefox → Core
The element isn't actually set as the target immediately when it's inserted in the DOM.  It's set that way when we actually try to perform the scroll to it.  In other words, what :target matches is "the element we most recently scrolled to".  Note that this is not the same thing as $(location.hash) from the jQuery issue discussion, because it really does depend on the scrolling behavior.  And in particular, what matches :target doesn't change when the DOM is modified but only when the actual scroll happends (during pageload, or due to location.hash changing after the page is loaded).

The problem during pageload, of course, is that there is no spec for when the browser should scroll to the target; it's all heuristics of various sorts...

Oh, and having :target always match $(location.hash) would make DOM mutations more expensive due to having to check, for every every node inserted/removed, whether it has an id matching location.hash (and if so, whether it's the first one in the DOM, but I assume matching would be rare anyway), so is not exactly desirable.

Note that there is an open spec issue on what the HTML spec should say for :target because when servo tried implementing what the spec says, as opposed to what Gecko and Blink do, they discovered that it's prohibitively expensive.  See https://github.com/whatwg/html/issues/190
(Reporter)

Comment 3

2 years ago
> having :target always match $(location.hash) would make DOM mutations more expensive due to having to check, for every every node inserted/removed, whether it has an id matching location.hash

You don't cache queries for `document.querySelector('#test')`, so what makes `:target` different? It seems silly to me that `:target` isn't treated as though it's equivalent to a templated version of `[id="test"]` where `test` gets replaced with whatever `location.hash` currently is.

That all said, I'm not asking for the CSS implementation of `:target` to change its behavior. I get that there may be restrictions that prevent `:target` styles from applying immediately, in the same way that I get that there may be restrictions that prevent `#test` styles from applying immediately.

What is unacceptable from a JS API standpoint is that the code to query the DOM for the `:target` element is unreliable and therefore unusable. If `:target` needs to be cached to make CSS work, that's fine, but any DOM manipulation will need to at least mark the cache as dirty so that a subsequent call to `document.querySelector` or `document.querySelectorAll` knows to refresh that particular value.

Any other behavior would mean that all code involving `:target` should be refactored to remove the unreliable selector. I've already had to do this, because there is no known point at which `:target` can be reliably selectable. Immediately after the element exists in the DOM is always too early. document.ready is usually too early. document.body.onload is sometimes too early.
(Reporter)

Comment 4

2 years ago
> having :target always match $(location.hash) would make DOM mutations more expensive due to having to check, for every every node inserted/removed, whether it has an id matching location.hash

You don't cache queries for `document.querySelector('#test')`, so what makes `:target` different? It seems silly to me that `:target` isn't treated as though it's equivalent to a templated version of `[id="test"]` where `test` gets replaced with whatever `location.hash` currently is.

That all said, I'm not asking for the CSS implementation of `:target` to change its behavior. I get that there may be restrictions that prevent `:target` styles from applying immediately, in the same way that I get that there may be restrictions that prevent `#test` styles from applying immediately.

What is unacceptable from a JS API standpoint is that the code to query the DOM for the `:target` element is unreliable and therefore unusable. If `:target` needs to be cached to make CSS work, that's fine, but any DOM manipulation will need to at least mark the cache as dirty so that a subsequent call to `document.querySelector` or `document.querySelectorAll` knows to refresh that particular value.

Any other behavior would mean that all code involving `:target` should be refactored to remove the unreliable selector. I've already had to do this, because there is no known point at which `:target` can be reliably selectable. Immediately after the element exists in the DOM is always too early. document.ready is usually too early. document.body.onload is sometimes too early.
> You don't cache queries for `document.querySelector('#test')`

Oh, we do.  Or more precisely, for every node that has an id, insertion/removal does in fact check for selectors that involve that id, updates the document's id-to-nodelist hashtable, etc.  I'm just against adding even more somewhat slow stuff on that codepath.

> `:target` isn't treated as though it's equivalent to a templated version of `[id="test"]`

It's not equivalent, though.  It has quite different behavior; c.f. <a name>.

And getting the current value of location.hash is not exactly cheap either.

> That all said, I'm not asking for the CSS implementation of `:target` to change its behavior.

Sure you are.  The selectors API is defined to match exactly the way CSS does.  If you're asking for the selectors API behavior to change, you're asking for the CSS behavior to change.
Or you're asking for some not clearly defined spec change to the selectors API, of course, to make it no longer match the way CSS does.
And to be even more clear, it's not that :target is _cached_ in some way.  It's that :target is state stored on a node, and that state gets stored when the browser actually scrolls to that target, not when location.hash is changed or the page is loaded.  That is, :target means "the thing the browser last scrolled to, if any", which is what it was intended to mean in terms of styling...
(Reporter)

Comment 8

2 years ago
Ok, so if it's poorly defined, what actions do I need to take to change the spec to change the browser implementation to convince the jQuery team that there's an issue to fix the selector to not be useless so that I can write compound selectors involving `:target` instead of `$(location.hash)`?
You could try contacting the CSS working group at www-style@w3.org or the HTML folks at whatwg@whatwg.org; they would need to coordinate on this.

That said, I suspect browsers are not likely to want to change here: scrolling to the target too early is NOT a good idea and applying the :target styles before you have scrolled is likely to cause undesired visual glitches in pages.
(Reporter)

Comment 10

2 years ago
> scrolling to the target too early is NOT a good idea and applying the :target styles before you have scrolled is likely to cause undesired visual glitches in pages.

I'm not asking the browser to scroll to the `:target` element, I'm asking the browser to select the `:target` element.

Why is it reasonable to expect that `document.querySelector('#test')` selects a DOM node, but it is unreasonable to expect that `document.querySelector(':target')` selects a DOM node for the sole reason that the styles have not yet been applied?

I can select `#test` before styles have been applied (for example, if the stylesheet hasn't loaded yet) why is the selection of `:target` special in this regard?

As a worst case, is there any reason that `:target` can't be guaranteed to be set by `window.onload`? Current implementations (I'll have to retest to remember whether it was Chrome or Firefox or both) sometimes still haven't figured out where the `:target` element is after the load event has fired.
You have it backwards.  The `:target` element is, by definition, the thing the browser has scrolled to.

Changing that, to be the thing it might scroll to in the future, has the "applying the :target styles before you have scrolled is likely to cause undesired visual glitches in pages" problem at the very least.

Matching the selector but not actually applying the styles is quite odd.  Matching the selector in some contexts but not others is also odd.  There's no good solution I see here.

> is there any reason that `:target` can't be guaranteed to be set by `window.onload`

It's not uncommon to modify the DOM during onload and not actually have the target element in the DOM until after the event is done firing.  That said, I expect that Firefox does in fact start matching :target before onload fires if the element is already in the DOM at that point.
You need to log in before you can comment on or make changes to this bug.