[webkit-reviews] review granted: [Bug 49278] Update image map lookup algorithm to match HTML5 specification : [Attachment 73419] 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 49278: Update image map lookup algorithm to match HTML5 specification
https://bugs.webkit.org/show_bug.cgi?id=49278

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

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=73419&action=review

> optimized for the normal case of an image with a single matching map element
in a way that was difficult to preserve with the correct HTML5 rule.

This sounds slightly disturbing - is this something to change in HTML5 perhaps?
Does Firefox implement the rule already?

r=me; please add test coverage for behavior changes mentioned in earlier
comments.

> WebCore/dom/Document.h:825
> -    void addElementById(const AtomicString& elementId, Element *element);
> -    void removeElementById(const AtomicString& elementId, Element *element);

> +    void addElementById(const AtomicString& id, Element*);
> +    void removeElementById(const AtomicString& id, Element*);

This isn't going to be a problem when including Document.h in an .mm file, is
it?

> LayoutTests/fast/images/image-map-multiple.html:77
> -    var numClicks = 7;
> +    var numClicks = 11;

I often make such tests with interdependent subtests myself, but I also often
feel bad about these. One really has to mentally interpret steps 1...10 to be
sure what's going on in step 11.