| Summary: | WKFrameIsDisplayingMarkupDocument should return true for Web Archives | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Antoine Quint <graouts> | ||||||
| Component: | New Bugs | Assignee: | Antoine Quint <graouts> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | commit-queue, ddkilzer, dino, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 114399 | ||||||||
| Attachments: |
|
||||||||
|
Description
Antoine Quint
2013-04-09 09:44:34 PDT
Created attachment 197138 [details]
Patch
Comment on attachment 197138 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=197138&action=review > Source/WebKit2/ChangeLog:8 > + Also return true for the Web Archive MIME type. Why? What are the implications of this change? Comment on attachment 197138 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=197138&action=review > Source/WebKit2/UIProcess/WebFrameProxy.cpp:118 > // FIXME: This returns false when displaying a web archive. Remove this comment. Created attachment 197149 [details]
Patch
(In reply to comment #5) > (From update of attachment 197138 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=197138&action=review > > > Source/WebKit2/ChangeLog:8 > > + Also return true for the Web Archive MIME type. > > Why? What are the implications of this change? This is so that commands in user agents that should only be contextual to markup documents do the right thing when a Web Archive is displayed. (In reply to comment #6) > (From update of attachment 197138 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=197138&action=review > > > Source/WebKit2/UIProcess/WebFrameProxy.cpp:118 > > // FIXME: This returns false when displaying a web archive. > > Remove this comment. Indeed! Comment on attachment 197149 [details]
Patch
It would be nice to know if there are any serious implications from this change.
One more concern. You said this was a regression. What was the commit that it regressed in? (In reply to comment #11) > One more concern. You said this was a regression. What was the commit that it regressed in? I believe it was a regression from WebKit1 to WebKit2. Comment on attachment 197149 [details] Patch Clearing flags on attachment: 197149 Committed r148078: <http://trac.webkit.org/changeset/148078> All reviewed patches have been landed. Closing bug. Comment on attachment 197149 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=197149&action=review > Source/WebKit2/UIProcess/WebFrameProxy.cpp:118 > + return m_MIMEType == "text/html" || m_MIMEType == "image/svg+xml" || m_MIMEType == "application/x-webarchive" || DOMImplementation::isXMLMIMEType(m_MIMEType); I’m not sure this patch is right. Can’t a web archive be created with contents that are not HTML, SVG, or XML? In that case, this function would return the wrong thing. I think in the case of the web archive we need an “internal MIME type” to correctly implement this function. This version might be OK for now, but there is a chance that it has some edge cases wrong. Comment on attachment 197149 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=197149&action=review >> Source/WebKit2/UIProcess/WebFrameProxy.cpp:118 >> + return m_MIMEType == "text/html" || m_MIMEType == "image/svg+xml" || m_MIMEType == "application/x-webarchive" || DOMImplementation::isXMLMIMEType(m_MIMEType); > > I’m not sure this patch is right. Can’t a web archive be created with contents that are not HTML, SVG, or XML? In that case, this function would return the wrong thing. I think in the case of the web archive we need an “internal MIME type” to correctly implement this function. > > This version might be OK for now, but there is a chance that it has some edge cases wrong. Yeah, I guess we need to see if the web archive has an item that we'd load as html or svg. I opened https://bugs.webkit.org/show_bug.cgi?id=114399 |