[webkit-reviews] review denied: [Bug 23452] Add XHTMLMP support to Webkit : [Attachment 27307] updated patch

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied yichao.yin
<yichao.yin at torchmobile.com.cn>'s request for review:
Bug 23452: Add XHTMLMP support to Webkit
https://bugs.webkit.org/show_bug.cgi?id=23452

Attachment 27307: updated patch
https://bugs.webkit.org/attachment.cgi?id=27307&action=review

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
The code here:

if (m_doc->isXHTMLDocument() && !m_hasDocTypeDeclaration) {

... bails if you don't have a DOCTYPE.	It only asks if you're an XHTML
document, so if XHTMLMP is enabled, you'll start being more strict in your
handling of XHTML documents that aren't also the XHTMLMP MIME type.  Is that
what you really want?

The code in parseDTD and externalSubsetHandler had lines like:

(publicId == QLatin1String("-//WAPFORUM//DTD XHTML Mobile 1.0//EN"))) {

These were unconditionally removed.  Shouldn't those just be left alone but put
inside #if !ENABLE(XHTMLMP)?

The new document method isXHTMLDocument() was added inside #if ENABLE(XHTMLMP).
 It's ok for that to be outside of the #ifdef.

Can you explain what the NOSCRIPT changes are about? Why do you need an actual
element for <noscript> in the XHTMLMP case?

Thanks!