| Summary: | Broaden support for mutation observation of attributes | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Adam Klein <adamk> | ||||||
| Component: | New Bugs | Assignee: | Adam Klein <adamk> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | darin, ojan, rafaelw, rniwa, sam, webkit.review.bot | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 68729 | ||||||||
| Attachments: |
|
||||||||
|
Description
Adam Klein
2011-12-13 14:18:23 PST
Created attachment 119085 [details]
Patch
Note to reviewer(s): the name "enqueueAttributesMutationRecordIfRequested" is rather...unfortunate. One possible fix is to rename it Element::willMutateAttribute, let me know if you'd refer that more general naming scheme. Comment on attachment 119085 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=119085&action=review > Source/WebCore/ChangeLog:25 > + (WebCore::Attr::setValue): Enqueue a mutation record when Attr.value is set from JS. > + (WebCore::Attr::childrenChanged): Enqueue a mutation record when an Attr's value changes to due additions/removals of Text children. > + * dom/Element.cpp: > + (WebCore::Element::enqueueAttributesMutationRecordIfRequested): Previously a static, expose as part of Element's interface to allow it to be re-used by NamedNodeMap and Attr. > + (WebCore::Element::removeAttribute): Remove enqueue call now handled by NamedNodeMap. > + (WebCore::Element::setAttributeInternal): Fixup call of enqueueAttributesMutationRecordIfRequested. > + * dom/Element.h: > + * dom/NamedNodeMap.cpp: > + (WebCore::NamedNodeMap::setNamedItem): Enqueue a mutation record when an attribute is changed via Element.attributes.setNamedItem from JS. > + (WebCore::NamedNodeMap::removeNamedItem): Enqueue a mutation record when an attribute is removed, either via Element.attributes.removeNamedItem or Element.removeAttribute. Please line-break as needed. These lines are extremely long. Doesn't even fit on my high-resolution 15" screen :( > LayoutTests/fast/mutation/observe-attributes.html:-9 > -<html> > -<head> > -<meta charset="utf-8"> > <script src="../js/resources/js-test-pre.js"></script> > -</head> > -<body> > -<p id=description></p> > -<div id="console"></div> Why are we removing these nodes? Comment on attachment 119085 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=119085&action=review >> LayoutTests/fast/mutation/observe-attributes.html:-9 >> -<div id="console"></div> > > Why are we removing these nodes? I've asked Rafael to remove these in other reviews. They don't affect the test in anyways, so they just add more noise. Comment on attachment 119085 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=119085&action=review >>> LayoutTests/fast/mutation/observe-attributes.html:-9 >>> -<div id="console"></div> >> >> Why are we removing these nodes? > > I've asked Rafael to remove these in other reviews. They don't affect the test in anyways, so they just add more noise. But they're auto-generated by the script, right? Also, not having body isn't desirable at times although it works in this test. Comment on attachment 119085 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=119085&action=review >> Source/WebCore/ChangeLog:25 >> + (WebCore::NamedNodeMap::removeNamedItem): Enqueue a mutation record when an attribute is removed, either via Element.attributes.removeNamedItem or Element.removeAttribute. > > Please line-break as needed. These lines are extremely long. Doesn't even fit on my high-resolution 15" screen :( Will break on submit. I feel like I heard at some point that breaking these lines screwed up trac's formatting, but I must be misremembering. Created attachment 119275 [details]
Patch for landing
Comment on attachment 119275 [details] Patch for landing Clearing flags on attachment: 119275 Committed r102814: <http://trac.webkit.org/changeset/102814> All reviewed patches have been landed. Closing bug. |