Add shifted element attribution (#11). #32
Conversation
index.bs
Outdated
| greater than the area of the <a>node impact region</a> of any | ||
| member of |S|. | ||
| 1. Return a {{FrozenArray}} containing one {{LayoutShiftAttribution}} object | ||
| for each member |s| of |S|, created by running the algorithm to |
There was a problem hiding this comment.
This seems undesirable because that means that those Nodes will cause memory leaks, as they cannot be destroyed when they are no longer needed by the user agent (say, when they are removed from the DOM tree but the Document remains alive).
|
Thanks for the review. Most of the comments are addressed in the latest update. Re. nodes being disconnected from the document, there are some aspects I am not sure about, perhaps we could sync up tomorrow to discuss. |
index.bs
Outdated
| Source attribution {#source-attribution} | ||
| ---------------------------------------- | ||
|
|
||
| In addition to the layout shift value, the API reports a sampling of up to five |
There was a problem hiding this comment.
This should be a non-normative note. It's also unclear who "we" is. Can you rephrase it?
There was a problem hiding this comment.
I was asked to remove the non-normative note text on the subsections because it was already applied to the entire "1. Introduction" section. Do you think each subsection should separately state it?
"We" in this case is me, the spec author :) How do you think it should be rephrased?
index.bs
Outdated
| ------------------------------------------------------------------ | ||
|
|
||
| <div algorithm="report the layout shift sources"> | ||
| When asked to <dfn>report the layout shift sources</dfn> for an active |
There was a problem hiding this comment.
Where is this algorithm called?
There was a problem hiding this comment.
This algorithm is called in step 9 of the algorithm to "report the layout shift" in subsection 5.1.
index.bs
Outdated
| When asked to <dfn export>report the layout shift sources</dfn> for an active | ||
| {{Document}} |D|, run the following steps: | ||
|
|
||
| 1. Let |S| be any set which satisfies these constraints: |
There was a problem hiding this comment.
Agree that a more explicit algorithm would be better - it won't necessarily constrain user agents in how they calculate the set (as long as it's not observably different from the algorithm), but will provide a baseline algorithm that they can then improve.
Thanks for addressing the feedback! Sure, we can discuss the nodes problem, feel free to add a sync with me and Yoav. |
|
I made a mess with my local branches. I'll start a new PR for this. |
💥 Error: write EPROTO 140222018242432:error:1407742E:SSL routines:SSL23_GET_SERVER_HELLO:tlsv1 alert protocol version:../deps/openssl/openssl/ssl/s23_clnt.c:772:
💥 ###
PR Preview failed to build. (Last tried on Mar 17, 2020, 12:36 AM UTC).
More
PR Preview relies on a number of web services to run. There seems to be an issue with the following one:
🚨 HTML Diff Service - The HTML Diff Service is used to create HTML diffs of the spec changes suggested in a pull request.
🔗 Related URL
If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.