w3ctag / design-reviews

W3C specs and API reviews
Creative Commons Zero v1.0 Universal
332 stars 56 forks source link

renderPriority element attribute #676

Closed vmpstr closed 2 years ago

vmpstr commented 3 years ago

Hello TAG!

I'm requesting a TAG review of renderPriority element attribute.

The renderPriority attribute is an HTML attribute that informs the User Agent to keep the element's rendering state updated with a specified priority. This is used on elements whose rendering state would not otherwise be kept up-to-date.

Further details:

We'd prefer the TAG provide feedback as (please delete all but the desired option):

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

andruud commented 3 years ago

The arguments for preferring a HTML attribute for this seem pretty weak so far, https://github.com/WICG/display-locking/issues/200#issuecomment-926450085. (Perhaps there are other unspoken arguments)?

vmpstr commented 3 years ago

The arguments for preferring a HTML attribute for this seem pretty weak so far, WICG/display-locking#200 (comment). (Perhaps there are other unspoken arguments)?

I left some more thoughts on that issue, but I would definitely appreciate TAG's view on it

kenchris commented 3 years ago

Will there be a way to set a default for this, say for a custom element?

LeaVerou commented 3 years ago

How does this work with nesting? E.g. if I have an ancestor with renderpriority="background" and a descendant with renderpriority="user-blocking", doesn't that effectively cancel the ancestor's renderpriority, since the ancestor would need to be rendered to render the descendant?

Wouldn't that also enable advertisers from just injecting HTML with renderpriority="user-blocking" to hog resources for their ads?

I would also have liked a discussion on what kind of performance gain would this enable, for a sufficiently complex DOM (e.g. the HTML spec). Are we increasing the complexity of the web platform to save some nanoseconds or is it a more significant gain?

An example use case is optimizing the speed of single-page-application navigations. If the application can predict a likely user action, then it can prerender the next view offscreen via content-visibility: hidden plus renderpriority. This will make the single-page application navigations faster for the user.

How would an author use renderpriority to enable this use case? What value would they use? Are they supposed to change the value before navigation?

vmpstr commented 3 years ago

Thank you for you feedback and questions!

Will there be a way to set a default for this, say for a custom element?

We don't have plans to expose a way to set a default behaviour. One of the alternatives that @andruud suggested is to use a CSS property instead of an Element attribute. This would allow you to target more than one element, effectively setting a default behaviour. As the issue talks about, I'd like to get TAG guidance on whether the attribute or property is more appropriate here.

How does this work with nesting? E.g. if I have an ancestor with renderpriority="background" and a descendant with renderpriority="user-blocking", doesn't that effectively cancel the ancestor's renderpriority, since the ancestor would need to be rendered to render the descendant?

Wouldn't that also enable advertisers from just injecting HTML with renderpriority="user-blocking" to hog resources for their ads?

I've had some thoughts on a similar case here: https://github.com/WICG/display-locking/issues/200#issuecomment-919645611 I agree that upgrading the renderpriority to something higher in the subtree is problematic, since it would violate the ancestor render priority setting and cause more work than necessary (as you mention with the ads injecting high priority content). I believe there is no such issues if we keep the priority at the minimum of ancestor and nested element (i.e. if one of them is background, then the content in the inner element is updated with background priority). We are also considering not updating nested renderpriority things at all (ie treat them as renderpriority=never), which would simplify reasoning about this and allow us to have improvements in the future.

I would also have liked a discussion on what kind of performance gain would this enable, for a sufficiently complex DOM (e.g. the HTML spec). Are we increasing the complexity of the web platform to save some nanoseconds or is it a more significant gain?

I can add a section to the explainer to try and foster such a discussion. Just from dealing with content-visibility content, I can say that it saves anywhere on the order from milliseconds to seconds of load time. The way this is set up though is that the work is deferred until it is needed. With appropriately chunked content, content-visibility: auto would naturally do this as the user scrolls, updating small pieces at a time as they enter the viewport. In the Single-Page App example that I alluded to, however, this means that whatever work you may save will still need to happen when the content is displayed to the user (and it would happen synchronously).

With this proposed feature, the developer can set renderpriority=background on the element which will ultimately be presented to the user. This will allow the user-agent to start doing work in that subtree without displaying the content, but preparing things like style and layout values. When the content is ultimately presented, the work that was initially deferred would have already been completed, so the content is displayed faster. So the savings here largely depend on the complexity of the content displayed and the amount of time the user-agent had to update this with background priority.

How would an author use renderpriority to enable this use case? What value would they use? Are they supposed to change the value before navigation?

The developer would display the content (i.e. removing content-visibility: hidden). Changing the renderpriority value is something that they can do optionally as well, but I don't think it's a necessity to get the performance.

LeaVerou commented 3 years ago

Thank you for the explanation. Saving milliseconds to several seconds certainly sounds worthwhile to me.

I agree that upgrading the renderpriority to something higher in the subtree is problematic, since it would violate the ancestor render priority setting and cause more work than necessary (as you mention with the ads injecting high priority content). I believe there is no such issues if we keep the priority at the minimum of ancestor and nested element (i.e. if one of them is background, then the content in the inner element is updated with background priority). We are also considering not updating nested renderpriority things at all (ie treat them as renderpriority=never), which would simplify reasoning about this and allow us to have improvements in the future.

Given that the default renderpriority is user-visible, wouldn't that effectively render user-blocking impossible to use, unless you're willing to set it on the root, defeating its purpose?

vmpstr commented 3 years ago

Given that the default renderpriority is user-visible, wouldn't that effectively render user-blocking impossible to use, unless you're willing to set it on the root, defeating its purpose?

Hmm, the default value is auto which allows the user-agent to select "whatever is appropriate". You're right that we need to be careful not to set a lower default (I'll update the explainer to call this out). Currently though, we're only proposing that this attribute has an effect only if rendering of the contents of that element would be skipped by the user-agent. From the explainer,

If the User Agent does not optimize rendering of elements, by skipping work, then the attribute has no effect.

The question is, of course, if it has no effect on the element, does it still affect the nested renderattribute settings. I don't think it should, but that's not (yet) clear in the explainer.

torgo commented 2 years ago

Hi @vmpstr - we're just reviewing progress on this today and it looks like so far the explainer hasn't been updated. Do you you have any additional status you can share?

vmpstr commented 2 years ago

Hey, I apologize for the delay. I've just updated the explainer as a result of the discussions that we had on this issue.

Thank you!

LeaVerou commented 2 years ago

Hi @vmpstr,

@cynthia and I discussed this again today in a breakout today. On attribute vs CSS property we definitely think this is more suitable as a CSS property. This way it is easier to use it for creating defaults, including in Web components, and also given that it interacts with other CSS properties.

We are still not sure about certain details of the proposed algorithm, including my question from above about nesting higher priority elements inside lower priority elements.

We would like to see some examples of actually using the feature to target the user needs you are targeting. Are all these values necessary to address these use cases?

vmpstr commented 2 years ago

Thank you for your feedback and questions!

On attribute vs CSS property we definitely think this is more suitable as a CSS property. This way it is easier to use it for creating defaults, including in Web components, and also given that it interacts with other CSS properties.

That makes sense, thanks.

We are still not sure about certain details of the proposed algorithm, including my question from above about nesting higher priority elements inside lower priority elements.

The intent is to have this property be treated as a maximum rendering priority for itself and its descendants. This would mean that higher priority descendants would still be limited by the lower priority ancestors. In other words, a parent with a background priority will cause all of its children to update with at most a background priority, even if one of its children is a user-blocking priority.

We would like to see some examples of actually using the feature to target the user needs you are targeting. Are all these values necessary to address these use cases?

I don't have any working examples since we don't have a working prototype of this feature, but I can work to construct hypothetical examples where this would be useful.

Thanks again!

torgo commented 2 years ago

closing for now based on @chrishtr - can be re-opened when appropriate