[Webkit-unassigned] [Bug 91228] imagemap without a "name" attribute doesn't work (affects xkcd.com)

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=91228


Alexey Proskuryakov <ap at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #165869|review?                     |review-
               Flag|                            |




--- Comment #9 from Alexey Proskuryakov <ap at webkit.org>  2012-09-26 14:03:24 PST ---
(From update of attachment 165869)
View in context: https://bugs.webkit.org/attachment.cgi?id=165869&action=review

I think that this patch can use another iteration, primarily for ChangeLog, but also potentially for some of the questions.

> Source/WebCore/ChangeLog:13
> +        Bug: Imagemap only looks for maps with natching names and disregards maps with matching
> +             id attributes.
> +
> +        Fix: Run the mapping code for cases in which the attribute is id as well. Additionally,
> +             in the case that name and id of different maps match the usemap argument, we map 
> +             to the first one encountered. This is the same strategy used by Opera and Firefox.

I think that this explanation can be shorter, and go into per-funtiton comment section below.

You say that this matches Opera and Firefox - what about IE?

Is this a change in HTML5 from earlier HTML specs?

> Source/WebCore/html/HTMLMapElement.cpp:107
> -        if (isIdAttributeName(attribute.name())) {
> +        if (isIdAttributeName(attribute.name()))

You still need the braces, as there is more than one line inside. We do count the comments.

> LayoutTests/ChangeLog:10
> +        * fast/loader/resources/beach-scene.png: Added.

What is the license for this image?

> LayoutTests/fast/loader/map-images-to-id-and-name.html:12
> +                eventSender.mouseMoveTo(400, 130);
> +                eventSender.mouseDown();
> +                eventSender.mouseUp();

Do we have to use EventSender? The downside is that the test won't work in browser. Did you check existing image map tests for techniques?

If EventSender is a must, please add an explanation to the test about how to run it manually.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.