According the DTD of XHTML Client-side Image Map Module loaded by the XHTML+RDFa DTD, an element map should not have to name attribute. In a valid XHTML+RDFa page, WebKit ignores the map that not declare a `name` attribute (only a `id` attribute). (Successfully tested on IE7+, Opera 12, Firefox 13).
Ted, do you know if this is something we mean to support?
I don't think it makes sense to change browser engines for RDFa. That said, WebKit deviates from the HTML spec in this case: § 4.8.14.2 (which defines the map element's processing model) says that the "rules for parsing a hash-name reference to a map element must be followed." http://www.whatwg.org/specs/web-apps/current-work/multipage/the-map-element.html#processing-model The rules for parsing a hash-name reference say to "[r]eturn the first element[…] that has an id attribute whose value is a case-sensitive match[…] or a name attribute whose value is a compatibility caseless match" http://www.whatwg.org/specs/web-apps/current-work/multipage/common-microsyntaxes.html#rules-for-parsing-a-hash-name-reference I don't think there would be a significant compat risk to such a change.
As mentioned by Ted, this is not related to XHTML, and fails in HTML in exactly the same manner. Per a bug I'm about to dupe, this affects xkcd.com.
*** Bug 97204 has been marked as a duplicate of this bug. ***
Duplicate has a suggested fix that may or may not be correct, I can't tell immediately.
(In reply to comment #5) > Duplicate has a suggested fix that may or may not be correct, I can't tell immediately. I did not suggest it as a fix, as I'm not sure about the original purpose of the "if (document()->isHTMLDocument())". It was meant only as a pointer to the relevant part of code. Thanks for looking into that!
<rdar://problem/12359382>
Created attachment 165869 [details] Patch
Comment on attachment 165869 [details] Patch 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.
(In reply to comment #9) > (From update of attachment 165869 [details]) > > > LayoutTests/ChangeLog:10 > > + * fast/loader/resources/beach-scene.png: Added. > > What is the license for this image? There are various pngs scattered throughout the LayoutTests directory - we have rights to use any of them, so you can substitute any of them. > > > 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. Existing <map> tests that test similar functionality do use eventSender. At least the ones that I could find. I think simulating an actual user click is relevant here. Akash, when Alexey mentions adding an explanation on how to run manually he simply means some text above the images that mentions the fact that if you're running the test manually you have to click on one map or the other, and what the expectation is when you do.
Created attachment 166037 [details] Patch
Comment on attachment 166037 [details] Patch Attachment 166037 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14036899 New failing tests: fast/loader/map-images-to-id-and-name.html fast/events/tabindex-focus-blur-all.html
Comment on attachment 166037 [details] Patch Attachment 166037 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14050373 New failing tests: fast/loader/map-images-to-id-and-name.html fast/events/tabindex-focus-blur-all.html
Comment on attachment 166037 [details] Patch Attachment 166037 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14068258 New failing tests: fast/loader/map-images-to-id-and-name.html fast/spatial-navigation/snav-imagemap-area-not-focusable.html http/tests/workers/terminate-during-sync-operation.html fast/spatial-navigation/snav-imagemap-overlapped-areas.html fast/events/tabindex-focus-blur-all.html fast/spatial-navigation/snav-imagemap-simple.html
Created attachment 167178 [details] Patch
Comment on attachment 167178 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=167178&action=review > Source/WebCore/html/HTMLMapElement.cpp:109 > + cachedValue = newValue; > + > +} Extra newline. > LayoutTests/ChangeLog:10 > + * fast/loader/map-images-to-id-and-name-expected.txt: Added. > + * fast/loader/map-images-to-id-and-name.html: Added. > + * fast/loader/resources/circle-and-square.png: Added. The test should go in fast/images
Created attachment 167200 [details] Patch
Comment on attachment 167200 [details] Patch Attachment 167200 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14175349 New failing tests: fast/images/image-map-multiple-xhtml.xhtml
Comment on attachment 167200 [details] Patch Attachment 167200 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14173410 New failing tests: fast/images/image-map-multiple-xhtml.xhtml
Comment on attachment 167200 [details] Patch Attachment 167200 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14172545 New failing tests: fast/images/image-map-multiple-xhtml.xhtml
Why did mac-ews try this patch twice?
> Why did mac-ews try this patch twice? There's a one-hour timeout. If we don't hear from the bot in an hour, we assume it died and will give the job to another bot. We might need to raise the timeout.
Should the patch here be committed or is the failing test meaningful?
Seems likely to be a real failure since it came up twice and seems related to the patch.
Comment on attachment 167200 [details] Patch Removing r+ since this appears to cause a material test failure. It would be nice to see a proper fix.
Also broke http://car.driver.jp/ in My Window 7, FF 22、IE 8/9、Opera 12.14，all support this "ref by id" behavor, except WebKit(including Chrome 28)
My understanding: 1、match name if <map> has name, or else match the id: this should mainly solve the problem! 2、if there are multiple <map> elements with the same name attribute, the spec say match the first? Well, i thought match depending on element's stroring order is not very good, the `match` procedure should be combined with 2 parts: (1)iterate over once within a (un-ordered?)container; (2)match each element and give a score, the scores should be strict-linear order, that is to say, any 2 scores is not equal, since if permitting euqal, then we have to take position/order info into match consideration.
http://www.w3.org/html/wg/drafts/html/master/embedded-content-0.html#the-map-element The name attribute gives the map a name so that it can be referenced. The attribute must be present and must have a non-empty value with no space characters. The value of the name attribute must not be a compatibility-caseless match for the value of the name attribute of another map element in the same document. If the id attribute is also specified, both attributes must have the same value. From the draft spec, we can say it's too strict. This does not fit the spirit of HTML5 -- improve error compatibility. If you say HTML5 allows more error-compatible parsing, then it should allows more loose name-id attribute matching, by the same principle? So my thought: (1)<img>'s usemap first matches <map>'s name, if there are multiple <map>'s with the same name, match the first; (2)Then if no <img> with the matched name, match against id. if there are multiple <map>'s with the same id, match the first; This would be more consistent and shows the clarity. The first modification in patch will violate the above rule: <img usemap="#b" .../> <map name="a" id="b" .../> <map name="b" id="c" .../> The `keyMatchesMapNameOrId` will cause the first <map> matched, while it may be better the second <map>. Or just modifier the parse? It'll be more simple: if <map> has no name but id, just add a name according to id.
The patch has an error: it can not match upper-cased Id attribute. inline bool keyMatchesLowercasedMapNameOrId(AtomicStringImpl* key, Element* element) { if (!element->hasTagName(mapTag)) return false; if (element->fastGetAttribute(nameAttr).lower().impl() == key) return true; > if (element->getIdAttribute().lower().impl() == key) return true; return false; }
The new case: //Test 2, 3 failed <html> <head> <script> function log(msg) { document.getElementById('console').appendChild(document.createTextNode(msg + "\n")); } function test(testNumber, success) { log("Test " + testNumber + ":") if (success) log(" PASS: Correct map selected."); else log(" FAIL: Incorrect map selected."); } </script> </head> <body> <div style="display:inline-block"> <p>Click on the blue squares for Tests 1 - 4. You should see "PASS" to the right for the first two, and "Hit me" for the next two</p> <p>Test 1</p> <img src="square-blue-100x100.png" usemap="#TEST1"> <map id="TEST1-not-the-same" name="TEST1"> <area shape="rect" coords="0,0,100,100" href="javascript:void(test(1, true))"> </map> <map name="TEST1"> <area shape="rect" coords="0,0,100,100" href="javascript:void(test(1, false))"> </map> <p>Test 2</p> <img src="square-blue-100x100.png" usemap="#TEST2"> <map id="TEST2" name="TEST2-not-the-same"> <area shape="rect" coords="0,0,100,100" href="javascript:void(test(2, true))"> </map> <map name="TEST2"> <area shape="rect" coords="0,0,100,100" href="javascript:void(test(2, false))"> </map> <p>Test 3</p> <img src="square-blue-100x100.png" usemap="#TEST3"> <p>Test 4</p> <img src="square-blue-100x100.png" usemap="#TEST4"> <map id="TEST3"> <area shape="rect" coords="0,0,100,100" href="javascript:void(test(3, true))"> </map> <map name="TEST4"> <area shape="rect" coords="0,0,100,100" href="javascript:void(test(4, true))"> </map> </div> <div style="float:right"> <pre style="margin-top:50px" id="console"></pre> </div> </body> </hmtl>
Fix: Node::InsertionNotificationRequest HTMLMapElement::insertedInto(ContainerNode* insertionPoint) { - if (insertionPoint->inDocument()) - treeScope()->addImageMap(this); + if (insertionPoint->inDocument()) { + updateCachedValue(m_cachedId, document()->isHTMLDocument() ? getIdAttribute().lower() : getIdAttribute(), treeScope(), this); + updateCachedValue(m_cachedName, document()->isHTMLDocument() ? fastGetAttribute(nameAttr).lower() : fastGetAttribute(nameAttr), treeScope(), this); + } return HTMLElement::insertedInto(insertionPoint); } When do match in ignoring case mode, the "Id" attribute also need to be lower()'ed. In fact, better use equalsIgnoringCase... This fixs http://car.driver.jp/ , :-)
Tests: https://github.com/w3c/web-platform-tests/pull/4511 (Note that the spec now requires case-sensitive matching, not compatibility caseless.)
Tests landed in web-platform-tests.
Safari Technology Preview 155 do still fail tests: http://wpt.live/html/semantics/embedded-content/image-maps/image-map-processing-model/hash-name-reference.html
https://wpt.fyi/results/html/semantics/embedded-content/image-maps/image-map-processing-model/hash-name-reference.html?label=master&label=experimental&product=chrome&product=firefox&product=safari&aligned&view=subtest shows a variety of tests failing cross-browser.
I have local patch based on Blink patch and it make us pass WPT test cases. Will try to do draft PR later today and see how it goes. Meanwhile assigning it to myself.
Forgot to do it in Draft but anyway - https://github.com/WebKit/WebKit/pull/14268 ^ Let's see. How it goes..