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 upImplement MutationObserver childList mutations. #16933
Conversation
highfive
commented
May 18, 2017
|
Heads up! This PR modifies the following files:
|
|
cc @jdm I was able to run some basic AFrame demos with the current implementation. Still have to do more tests because more complex demos crash due to a GC issue (it isn't related to mutation observers). I might need disconnect method implementation too. |
|
Great work! My only comment is that we should be queueing the mutation records at the points that matches the relevant specification algorithm, rather than in children_changed. |
| @@ -2449,6 +2449,58 @@ impl VirtualMethods for Node { | |||
| if let Some(list) = self.child_list.get() { | |||
| list.as_children_list().children_changed(mutation); | |||
| } | |||
|
|
|||
| // Queue Mutation Observer records | |||
| match *mutation { | |||
This comment has been minimized.
This comment has been minimized.
jdm
May 18, 2017
Member
I appreciate how nice it is to group these together here, but we really should be calling queue_a_mutation_record from places like https://dxr.mozilla.org/servo/rev/d1eef9d08a4dc3c7ff057797281d2b4f8d4f71c7/components/script/dom/node.rs#1619 instead. This will match https://dom.spec.whatwg.org/#concept-node-insert step 4 and step 7 better, for example.
|
@MortimerGoro Please file that crash as a separate issue. I am extremely interested in all JS engine crashes. |
MortimerGoro
force-pushed the
MortimerGoro:mutation_children
branch
from
5a58939
to
0290e40
Done!
Ok, will create the issue ;) |
|
@bors-servo: r+ |
|
|
|
|
|
|
|
@bors-servo: retry |
Implement MutationObserver childList mutations. <!-- Please describe your changes on the following line: --> Implement MutationObserver childList mutations --- <!-- 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 - [ ] These changes fix #__ (github issue number if applicable). <!-- Either: --> - [x] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- 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. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16933) <!-- Reviewable:end -->
|
|
MortimerGoro commentedMay 18, 2017
•
edited by Manishearth
Implement MutationObserver childList mutations
./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThis change is