HTML5 specification says we should ignore usemap attributes without #. http://www.w3.org/TR/html5/infrastructure.html#valid-hash-name-reference
Created attachment 232166 [details] Patch
Comment on attachment 232166 [details] Patch Attachment 232166 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5169700786929664 New failing tests: fast/images/image-map-multiple-xhtml.xhtml
Created attachment 232171 [details] Archive of layout-test-results from webkit-ews-06 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-06 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 232166 [details] Patch Attachment 232166 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/4660278742482944 New failing tests: fast/images/image-map-multiple-xhtml.xhtml
Created attachment 232174 [details] Archive of layout-test-results from webkit-ews-05 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-05 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Created attachment 232175 [details] Patch
Created attachment 232176 [details] Patch
Comment on attachment 232176 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=232176&action=review > Source/WebCore/dom/TreeScope.cpp:210 > + if (m_rootNode.document().isHTMLDocument() && hashPos == notFound) > + return nullptr; What do other browsers do? I'm very afraid that this is not a backwards compatible change.
Comment on attachment 232176 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=232176&action=review >> Source/WebCore/dom/TreeScope.cpp:210 >> + return nullptr; > > What do other browsers do? I'm very afraid that this is not a backwards compatible change. FireFox and IE.10 does not support usemap attribute if it has not '#'. Blink works as WebKit as of now.
Comment on attachment 232176 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=232176&action=review >>> Source/WebCore/dom/TreeScope.cpp:210 >>> + return nullptr; >> >> What do other browsers do? I'm very afraid that this is not a backwards compatible change. > > FireFox and IE.10 does not support usemap attribute if it has not '#'. Blink works as WebKit as of now. Why specific to HTML documents? That part seems wrong to me. > Source/WebCore/dom/TreeScope.cpp:211 > String name = (hashPos == notFound ? url : url.substring(hashPos + 1)).impl(); If we’re exiting when hashPos is notFound, then we should be removing the ? : expression here.
Comment on attachment 232176 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=232176&action=review >>>> Source/WebCore/dom/TreeScope.cpp:210 >>>> + return nullptr; >>> >>> What do other browsers do? I'm very afraid that this is not a backwards compatible change. >> >> FireFox and IE.10 does not support usemap attribute if it has not '#'. Blink works as WebKit as of now. > > Why specific to HTML documents? That part seems wrong to me. Given the many years of support for this in WebKit, it’s likely that some WebKit-specific content is accidentally depending on the old behavior. We’ll have to find some way of researching that question.
Created attachment 232368 [details] Patch Applied Darin's comments.
Comment on attachment 232176 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=232176&action=review >> Source/WebCore/dom/TreeScope.cpp:211 >> String name = (hashPos == notFound ? url : url.substring(hashPos + 1)).impl(); > > If we’re exiting when hashPos is notFound, then we should be removing the ? : expression here. I also agree that we need a way to handle this kind of issues. Specially, if there it is impossible to provide the legacy behavior like this, how can we apply the latest spec to the implementation?
r=me if we believe websites don't rely on this feature.
Comment on attachment 232368 [details] Patch Clearing flags on attachment: 232368 Committed r172796: <http://trac.webkit.org/changeset/172796>
All reviewed patches have been landed. Closing bug.