search

w3c / largest-contentful-paint

Specification for the LargestContentfulPaint API
https://w3c.github.io/largest-contentful-paint/
Other
88 stars 16 forks source link

Modifications to the DOM specification look like they won't work #18

Closed foolip closed 5 years ago

foolip commented 5 years ago

https://wicg.github.io/largest-contentful-paint/#sec-modifications-DOM proposes modifications to https://dom.spec.whatwg.org/#concept-event-dispatch that check if "target is a Window object".

However, this only works if the Window object is the target of the event, and in quick testing it seems like the Document object is the target. The event bubbles to the Window object.

Which spec dispatches "scroll" events? Looking at that might give a robust way of doing this bookkeeping.

cc @annevk for DOM modification.

npm1 commented 5 years ago

I think this should be 'if the target's relevant global object is a Window object' @yoavweiss ?

yoavweiss commented 5 years ago

I looked at step 2 of said DOM algorithm, which is "Let targetOverride be target, if legacy target override flag is not given, and target’s associated Document otherwise. [HTML]"

It's also followed by a note saying "legacy target override flag is only used by HTML and only when target is a Window object."

I took that to mean that if the legacy flag is passed on, target is a Window object, and if it isn't, target has an "associated Document", so it's a Window object. But maybe I misunderstood what target is after step 1 there.

foolip commented 5 years ago

How is this implemented in Chromium? Whatever that code does is probably a good starting point.

yoavweiss commented 5 years ago

I took that to mean that if the legacy flag is passed on, target is a Window object, and if it isn't, target has an "associated Document", so it's a Window object.

I think I was reading this wrong and in the non-legacy case, target doesn't have to be a Window. I'll fix it.