Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upmake Document.getElementsByName a live collection #25428
Conversation
highfive
commented
Jan 3, 2020
|
Heads up! This PR modifies the following files:
|
|
@bors-servo try=wpt |
make Document.getElementsByName a live collection Another new case for NodeList; this and the labels live collection in #25424 are in the pipeline simultaneously, so one of them will need a merge resolution when the other one lands. Iterating over many same-named elements is potentially slower than it has to be, since getting the nth element from the live view always starts from the start of the tree. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #25147 <!-- Either: --> - [X] There are tests for these changes <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
|
|
|
|
|
Also, #22264 (comment) may apply to this PR too? |
|
@asajeffrey: Yes to both questions, and indeed #22264 was an attempt to do it that way. I think #22264 might have been successful for the .getElementsByName subcase and only ran into trouble in the .labels case, so I could look into using that work here. |
|
I don't think I understand everything #22264 was doing with mutation observers. I understand the part within document.rs where it's using a mutation observer message to know when to mark the list stale, but I don't follow the changes to mutationobserver.rs. I might just need a fresher look in the morning though. |
|
@pshaughn What's the status here? Is this still waiting on a review? |
|
I don't know how to make it fast. If slow and spec-compliant is better than fast and not-spec-compliant, then this is ready to review; if not then this should just be closed. |
|
I am inclined to start with slow and correct, since that gets us closer to web compatibility than no implementation at all. |
|
@bors-servo r+ |
|
|
make Document.getElementsByName a live collection Another new case for NodeList; this and the labels live collection in #25424 are in the pipeline simultaneously, so one of them will need a merge resolution when the other one lands. Iterating over many same-named elements is potentially slower than it has to be, since getting the nth element from the live view always starts from the start of the tree. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #25147 <!-- Either: --> - [X] There are tests for these changes <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
|
|
|
@bors-servo retry
|
|
|
pshaughn commentedJan 3, 2020
Another new case for NodeList; this and the labels live collection in #25424 are in the pipeline simultaneously, so one of them will need a merge resolution when the other one lands.
Iterating over many same-named elements is potentially slower than it has to be, since getting the nth element from the live view always starts from the start of the tree.
./mach build -ddoes not report any errors./mach test-tidydoes not report any errors