Summary: | Ignore usemap attributes without '#' in img element | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jinwoo Song <jinwoo7.song> | ||||||||||||||
Component: | WebCore Misc. | Assignee: | Jinwoo Song <jinwoo7.song> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | buildbot, commit-queue, esprehn+autocc, kangil.han, rniwa | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Attachments: |
|
Description
Jinwoo Song
2014-05-27 21:30:03 PDT
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. |