[Webkit-unassigned] [Bug 23452] Add XHTMLMP support to Webkit

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=23452


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #27559|review?                     |review-
               Flag|                            |




------- Comment #12 from darin at apple.com  2009-02-11 06:24 PDT -------
(From update of attachment 27559)
> +#if ENABLE(XHTMLMP)
> +    ASSERT(settings());
> +    m_shouldProcessNoScriptElement = !settings()->isJavaScriptEnabled();
> +#endif

I don't think this assertion is helpful. We don't want to assert ever pointer
just before we dereference it.

But I also don't know if settings() is guaranteed to be non-zero here or not.
Is it guaranteed?

> +    return (frame()->loader()->responseMIMEType() == "application/vnd.wap.xhtml+xml" || frame()->loader()->responseMIMEType() == "application/xhtml+xml");

Please leave out the parentheses.

> +    }
> +#endif
> +   m_sawFirstElement = true;

Indentation is wrong here.

> +#if ENABLE(XHTMLMP)
> +static const char* const defaultHttpAcceptHeader = "application/xml,application/vnd.wap.xhtml+xml,application/xhtml+xml;profile='http://www.wapforum.org/xhtml',text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5";
> +#else
> +static const char* const defaultHttpAcceptHeader = "application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5";
> +#endif

This is good. static const char defaultHTTPAcceptHeader[] would be even better,
I think. WebKit style capitalizes the acronym "HTTP", and doesn't use "Http".
Or you can leave it out of this name altogether. I don't think it's that
confusing to call this just an "accept header" locally in this file.

> +#if PLATFORM(QT) && ENABLE(XHTMLMP)
> +    // FIXME: Qt is flawed
> +    // This hacking is for fixing one bug of javasrcipt in XHTMLMP document, 
> +    // i.e: if alert() method is contained directly in <script> element, 
> +    // it will cause QtLauncher crash when frame loader execute it.
> +    if (m_isRunningScript && m_frame->document() && m_frame->document()->isXHTMLMPDocument())
> +        return;
> +#endif

We need to figure out the real problem and fix this in a better way.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.