[webkit-reviews] review granted: [Bug 39692] Partly done support for alternate ID attributes should be removed : [Attachment 57481] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Darin Adler <darin at apple.com>'s
request for review:
Bug 39692: Partly done support for alternate ID attributes should be removed
https://bugs.webkit.org/show_bug.cgi?id=39692

Attachment 57481: Patch
https://bugs.webkit.org/attachment.cgi?id=57481&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
> -	   const QualifiedName& attributeName = (attrName == idAttr) ?
e->idAttributeName() : attrName;
> -	   if (e->isEnumeratable() && e->getAttribute(attributeName) == name) {

> +	   if (e->isEnumeratable() && e->getAttribute(attrName) == name) {

I don't see this change explained in ChangeLog, and I'm not sure if I
understand it.

>      RenderSVGResourceContainer(SVGStyledElement* node)
>	   : RenderSVGHiddenContainer(node)
>	   , RenderSVGResource()
> -	   , m_id(node->getIDAttribute())
> +	   // FIXME: Should probably be using getIdAttribute rather than
idForStyleResolution.
> +	   , m_id(node->hasID() ? node->idForStyleResolution() : nullAtom)

The FIXME doesn't make it immediately clear why (it's also not always clear
elsewhere, but this instance particularly caught my eye, because this is
rendering code, so it's kind of closer to "for style resolution").

r=me