Element.checkVisibility review

[vmpstr]

Hello,

I'm requesting a TAG review of Element.isVisible.

Element.isVisible() returns true if the element is visible, and false if it is not. It checks a variety of factors that would make an element invisible, including visibility, content-visibility, and opacity.

• Explainer: https://github.com/WICG/display-locking/blob/main/explainers/isvisible.md

• Specification URL: https://drafts.csswg.org/cssom-view/#dom-element-checkvisibility

• Security and Privacy self-review: https://github.com/WICG/display-locking/blob/main/privacy-assessments/isvisible.md

• Primary contacts (and their relationship to the specification):

  • @josepharhar (Google, implementor)
  • @tabatkins (Google, spec author)
  • @vmpstr (Google)

• Organization(s)/project(s) driving the specification: Google, Chromium

• Key pieces of existing multi-stakeholder review or discussion of this specification: https://github.com/w3c/csswg-drafts/issues/6850

• External status/issue trackers for this specification (publicly visible, e.g. Chrome Status): https://chromestatus.com/feature/5163102852087808

Further details:

• [x] I have reviewed the TAG's Web Platform Design Principles
• Relevant time constraints or deadlines: None, but this feature assists with successful adoption of content-visibility: hidden
• The group where the work on this specification is currently being done: CSSWG

• This work is being funded by: Google

  💬 leave review feedback as a comment in this issue and @-notify @vmpstr

[vmpstr]

For additional info, we're still discussing a good name for this function. The bulk of the discussion can be found in this issue https://github.com/w3c/csswg-drafts/issues/7317

[plinss]

Has there been any discussion of making this async? In general we'd prefer not to add more functionality that synchronously blocks for style computation (or layout).

[atanassov]

Also, despite the use cases being somewhat obvious, we encourage you to write a complete explainer and capture all the other important bits in a single place - other considerations, code examples, privacy, security and a11y considerations etc. Can you please write one?

[josepharhar]

There is already an explainer and privacy/security self review linked in the issue description. Is there something missing in those?

[vmpstr]

Has there been any discussion of making this async? In general we'd prefer not to add more functionality that synchronously blocks for style computation (or layout).

We haven't considered making this asynchronous. IMHO it would complicate the API and code using it, to avoid a potential style (+ possibly layout) update. If my reading of https://www.w3.org/TR/design-principles/#synchronous is correct, the general preference is for synchronous APIs unless there is substantial I/O or heavy work. Again IMHO, I don't think style and layout qualify here. There is already a number of APIs like getBoundingClientRect that have to process style + layout. In other words, a number of existing APIs already work as if style and layout is always up-to-date from script perspective. I can't see a justification why this API should be different.

[plinss]

The reason I ask is that the general consensus is having synchronous mechanisms that force style computation and layout were a mistake. If we were designing the DOM APIs today those would all be promise based.

Now, one can argue that horse has left the stable, and adding one more synchronous method is relatively harmless, and even better for developers as they wouldn't be faced with a mix of sync/async methods.

On the other hand, many of us would like to see new async methods to access that data, and all the existing synchronous methods and properties become deprecated. Going down that path, adding more and more synchronous methods makes the eventual transition to async more complex.

I don't believe the TAG is going to reject this for being synchronous, but we'd like to see that the option has at least been taken into consideration by the relevant WGs. We'd also like to see some traction on developing async replacements for the various other APIs that force style computation and layout. (Were those to be under active development, it would make more sense for new mechanisms to follow that path and lead authors into the async world.)

[vmpstr]

Sorry for the late reply.

Without considering existing features, I still believe that this function should be synchronous. The intended use-case for this feature is for the developer to check the existing element with current styles for visibility to see if a further measurement is appropriate. Something like the following:

if (e.checkVisibility())
  return e.getBoundingClientRect();
else
  return null;

The reason this pattern is useful is the way content-visibility: hidden, for example, works: if one just invokes e.getBoundingClientRect() and e is in a subtree of a content-visibility: hidden element then that will cause an update in an otherwise skipped subtree. The checkVisibility function, on the other hand, would only update the rendering while still respecting skipped elements, then can check that e is indeed under a content-visibility: hidden and return false

Having this function be asynchronous puts a burden on the developer to ensure that the function is either called in an async function with an await, or to restructure the 'natural' flow to account for the fact that the visibility check will happen at a future time. It isn't clear to me whether the guidance to prefer asynchronous APIs to avoid forced rendering updates should take precedence. It is my humble opinion that in this case it should not, and the function call should remain synchronous.

[LeaVerou]

The reason this pattern is useful is the way content-visibility: hidden, for example, works: if one just invokes e.getBoundingClientRect() and e is in a subtree of a content-visibility: hidden element then that will cause an update in an otherwise skipped subtree. The checkVisibility function, on the other hand, would only update the rendering while still respecting skipped elements, then can check that e is indeed under a content-visibility: hidden and return false

We had some trouble following this, how does making the API async complicate this use case?

Having this function be asynchronous puts a burden on the developer to ensure that the function is either called in an async function with an await, or to restructure the 'natural' flow to account for the fact that the visibility check will happen at a future time.

This applies to any async API, doesn’t it?

[vmpstr]

We had some trouble following this, how does making the API async complicate this use case?

Consider something like

function getSize(e) {
  if (e.checkVisibility())
    return e.getBoundingClientRect();
  else
    return new DOMRect();
}

function foo() {
  /* gather some information about the current DOM state */
  ...
  let size = getSize(e);
  ...
  /* do something with the information and size */
  ...
}

Here, getSize function has been updated to use checkVisibility; it used to (for sake of argument) just return getBoundingClientRect, which used to cause unnecessary rendering work. This now costs some necessary rendering work but no layout is needed for c-v hidden cases.

This is a correct update that affects one function because of synchronous nature of the API.

Now in order to update this same code but to use an asynchronous function, we have two options: await and promise.then. With await, this looks something like the following

async function getSize() {
  let is_visible = await e.checkVisibility();
  if (is_visible)
    return e.getBoundingClientRect();
  else
    return new DOMRect();
}

async function foo() {
  /* gather some information about the current DOM state */
  ...
  let size = await getSize();
  ...
  /* do something with the information and size,
     however the information we got about the current DOM state may no longer be correct,
     because getSize did asynchronous work which could have resulted in rAF callbacks, for example,
     modifying DOM state. */
  ...
}

Here the information we gathered may be out of date, and we need to further restructure the code, if possible, to still be correct. We also need to mark foo as async, which propagates further up the stack

The promise.then pattern definitely needs more refactoring here on top of getting new state.

Now, I'm not arguing that any of this is impossible to do. My only concern is that I don't see a compelling reason to force this complexity and async considerations on the developer when the synchronous version of the API seems to have no downsides, other than ensuring style is up to date (which is something that getBoundingClientRect would have done anyway)

This applies to any async API, doesn’t it?

Yes. I was highlighting that (IMHO) it seems unnecessary to add this requirement for this particular API.

FWIW, I am be missing some aspect that the asynchronous version provides, but if my understanding is correct, it seems like the synchronous version is more flexible here

[torgo]

@vmpstr we're not clear on the current status of this and the info in Chromestatus is at odds with the info in the request. Also the explainer is in an archived repo. Is there current work still going on and are you still seeking TAG review? If so, can you give us an update and also update the issue as well please?

[josepharhar]

I updated the chromestatus, we shipped this in chrome 105.

[torgo]

Thanks for that @josepharhar. We discussed again in today's TAG plenary call. Regarding multi-stakeholder support, we also note from caniuse data that there are multiple implementations, which is also not reflected in Chromestatus. Can you please update, or maybe start using the BCD data which powers CanIUse to also power this indication? We still have some reservations regarding sync vs. async as plinss highlighted however we are going to move that discussion to our Design Principles work.