RESOLVED FIXED 74448
Broaden support for mutation observation of attributes 
https://bugs.webkit.org/show_bug.cgi?id=74448
Summary Broaden support for mutation observation of attributes
Adam Klein
Reported 2011-12-13 14:18:23 PST
Broaden support for mutation observation of attributes
Attachments
Patch (20.24 KB, patch)
2011-12-13 14:26 PST, Adam Klein 		no flags
DetailsFormatted DiffDiff
Patch for landing (19.59 KB, patch)
2011-12-14 12:23 PST, Adam Klein 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Adam Klein
Comment 1 2011-12-13 14:26:06 PST
Created attachment 119085 [details] Patch
Adam Klein
Comment 2 2011-12-13 14:35:55 PST
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.
Ryosuke Niwa
Comment 3 2011-12-14 01:36:36 PST
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?
Ojan Vafai
Comment 4 2011-12-14 09:18:09 PST
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.
Ryosuke Niwa
Comment 5 2011-12-14 11:14:30 PST
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.
Adam Klein
Comment 6 2011-12-14 11:50:36 PST
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.
Adam Klein
Comment 7 2011-12-14 12:23:51 PST
Created attachment 119275 [details] Patch for landing
WebKit Review Bot
Comment 8 2011-12-14 13:14:33 PST
Comment on attachment 119275 [details] Patch for landing Clearing flags on attachment: 119275 Committed r102814: <http://trac.webkit.org/changeset/102814>
WebKit Review Bot
Comment 9 2011-12-14 13:14:38 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.