Bug 32706

Summary: <noscript> is taking effect even though the Javascript is enabled, when XHTMLMP is enabled
Product: WebKit Reporter: Charles Wei <charles.wei>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: charles.wei, commit-queue, dbates, staikos, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
patch
manyoso: review-
Layout tests
none
another patch none

Description Charles Wei 2009-12-18 03:19:43 PST
<noscript> is designed for browsers that doesn't support Javascript , or Javascript is  disabled.   But It's working even when Javascript is enabled with WebKit,  this happens when XHTMLMP is enabled.

to verify ,  use the following html snippets:

<html>
<noscript>
    <meta http-equiv=refresh content="0; URL=http://www.google.com" />
</noscript>

<body>
    If you see this text and not re-directed to google when your browser is Javascript enabled, it succeeds,  otherwise it fails .
</body>
</html>
Comment 1 Charles Wei 2009-12-20 23:46:47 PST
Created attachment 45308 [details]
patch

This patch fixes the problem "contents inside <noscript> is taking effect even when Javascript is enabled, this happens only when XHTMLMP is enabled".

<noscript> is enabled when XHTMLMP is enabled,   but the implementation only hides the contents inside <noscript> when Javascript is turned on, which is incorrect.

The correct solution is to skip the parsing of all the contents inside <noscript> if Javascript is turned on .
Comment 2 WebKit Review Bot 2009-12-20 23:49:18 PST
style-queue ran check-webkit-style on attachment 45308 [details] without any errors.
Comment 3 Adam Treat 2009-12-21 06:13:02 PST
Comment on attachment 45308 [details]
patch

Can you programmatically turn on/off JavaScript and test rather than just assume it is on and only test that case?  Also it isn't clear to me why the change to HTMLParser should be XHTMLMP only... If JS is on, then noscript should be skipped regardless of whether XHTMLMP is enabled or not, right? Also what about the previous changes to HTMLNoSCriptElement?
Comment 4 Daniel Bates 2009-12-22 16:50:09 PST
Created attachment 45411 [details]
Layout tests

A DRT test that checks that both <meta> and <script> are not processed within <noscript> under XHTML-MP.
Comment 5 Charles Wei 2009-12-22 19:44:37 PST
Created attachment 45415 [details]
another patch

Another patch that addresses the comments of the reviewer
Comment 6 WebKit Review Bot 2009-12-22 19:48:50 PST
style-queue ran check-webkit-style on attachment 45415 [details] without any errors.
Comment 7 Eric Seidel (no email) 2009-12-22 20:25:24 PST
Comment on attachment 45308 [details]
patch

Clearing cq+ on this obsolete patch.
Comment 8 Eric Seidel (no email) 2009-12-22 22:43:13 PST
Comment on attachment 45415 [details]
another patch

The change looks fine, but I can't understand what you're trying to say in the ChangeLog.
Comment 9 Daniel Bates 2009-12-23 10:13:00 PST
(In reply to comment #1)
> The correct solution is to skip the parsing of all the contents inside
> <noscript> if Javascript is turned on .

Note, by section 9.2.1.2 of the XHTML-MP 1.1 spec., we must process <noscript> even when scripting is enabled so long as we have encountered an script with an unsupported scripting language <http://www.openmobilealliance.org/Technical/release_program/docs/Browsing/V2_2-20061020-A/OMA-WAP-XHTMLMP-V1_1-20061020-A.pdf>.
Comment 10 Daniel Bates 2009-12-23 11:05:10 PST
(In reply to comment #0)
> <noscript> is designed for browsers that doesn't support Javascript , or
> Javascript is  disabled.   But It's working even when Javascript is enabled
> with WebKit,  this happens when XHTMLMP is enabled.
> 
> to verify ,  use the following html snippets:
> 
> <html>
> <noscript>
>     <meta http-equiv=refresh content="0; URL=http://www.google.com" />
> </noscript>
> 
> <body>
>     If you see this text and not re-directed to google when your browser is
> Javascript enabled, it succeeds,  otherwise it fails .
> </body>
> </html>

I am unclear from your snippet whether this is a valid XHTML-MP document and whether this document would or should be interpreted as being an XHTML-MP document. I mean, it's missing a DOCTYPE and an xml namespace. So, how does the browser know to interpret this as XHTML-MP? by file extension? by MIME type "application/vnd.wap.xhtml+xml"? when XHTML-MP is enabled in WebKit do we just interpret all documents as XHTML-MP? some combination?

(I briefly looked through some of the XHTML-MP code, and from my understanding, it looks for the DOCTYPE and xml namespace declarations as well as accepts the MIME type "application/vnd.wap.xhtml+xml".)

I would suggest only using the XHTML-MP logic when we clearly know that the document we received is an XHTML-MP document since by the HTML 5 spec. <noscript> has a precisely defined behavior under HTML, but has no meaning under XHTML (*). Hence, the expected result of interpreting your example as XHTML should be a redirect (regardless of whether JavaScript is enabled). And the expected result of interpreting the document as HTML with JavaScript enabled is no redirect.

Assuming JavaScript is enabled, then interpreting your example as an XHTML-MP document should not cause a redirect. But, consider the slightly modified example:

<html>
 <script type="unsupported-script-lang">...</script>
 <noscript>
     <meta http-equiv=refresh content="0; URL=http://www.google.com" />
 </noscript>

 <body>
     If you see this text and not re-directed to google when your browser is
 Javascript enabled, it succeeds,  otherwise it fails .
 </body>
</html>

Assume JavaScript is enabled and suppose this is interpreted as being an XHTML-MP document. Then by section 9.2.1.2 of the XHTML-MP 1.1 spec.<http://www.openmobilealliance.org/Technical/release_program/docs/Browsing/V2_2-20061020-A/OMA-WAP-XHTMLMP-V1_1-20061020-A.pdf>, we must process the <noscript> since we first encountered a <script> with an unsupported scripting language (i.e. "unsupported-script-lang"). So, the expected result is a redirect.

(*) Section 4.3.2 of the HTML 5 spec. states "...the noscript element is handled differently by the HTML parser based on whether scripting was enabled or not when the parser was invoked....The noscript element is only effective in the HTML syntax, it has no effect in the XHTML syntax" (http://dev.w3.org/html5/spec/Overview.html#the-noscript-element).
Comment 11 Charles Wei 2009-12-24 06:50:06 PST
> I am unclear from your snippet whether this is a valid XHTML-MP document and
> whether this document would or should be interpreted as being an XHTML-MP
> document. I mean, it's missing a DOCTYPE and an xml namespace. So, how does the
> browser know to interpret this as XHTML-MP? by file extension? by MIME type
> "application/vnd.wap.xhtml+xml"? when XHTML-MP is enabled in WebKit do we just
> interpret all documents as XHTML-MP? some combination?
> 
> (I briefly looked through some of the XHTML-MP code, and from my understanding,
> it looks for the DOCTYPE and xml namespace declarations as well as accepts the
> MIME type "application/vnd.wap.xhtml+xml".)
> 
> I would suggest only using the XHTML-MP logic when we clearly know that the
> document we received is an XHTML-MP document since by the HTML 5 spec.
> <noscript> has a precisely defined behavior under HTML, but has no meaning
> under XHTML (*). Hence, the expected result of interpreting your example as
> XHTML should be a redirect (regardless of whether JavaScript is enabled). And
> the expected result of interpreting the document as HTML with JavaScript
> enabled is no redirect.
> 
> Assuming JavaScript is enabled, then interpreting your example as an XHTML-MP
> document should not cause a redirect. But, consider the slightly modified
> example:
> 
> <html>
>  <script type="unsupported-script-lang">...</script>
>  <noscript>
>      <meta http-equiv=refresh content="0; URL=http://www.google.com" />
>  </noscript>
> 
>  <body>
>      If you see this text and not re-directed to google when your browser is
>  Javascript enabled, it succeeds,  otherwise it fails .
>  </body>
> </html>
> 
> Assume JavaScript is enabled and suppose this is interpreted as being an
> XHTML-MP document. Then by section 9.2.1.2 of the XHTML-MP 1.1
> spec.<http://www.openmobilealliance.org/Technical/release_program/docs/Browsing/V2_2-20061020-A/OMA-WAP-XHTMLMP-V1_1-20061020-A.pdf>,
> we must process the <noscript> since we first encountered a <script> with an
> unsupported scripting language (i.e. "unsupported-script-lang"). So, the
> expected result is a redirect.
> 
> (*) Section 4.3.2 of the HTML 5 spec. states "...the noscript element is
> handled differently by the HTML parser based on whether scripting was enabled
> or not when the parser was invoked....The noscript element is only effective in
> the HTML syntax, it has no effect in the XHTML syntax"
> (http://dev.w3.org/html5/spec/Overview.html#the-noscript-element).


Thanks, Daniel,  for the comment. 

This bug is introduced by XHTML-MP for HTML document -- when XHTML-MP is enabled, it's causing regressions for HTML documents , and that's what my patch tries to fix.

When XHTML-MP is enabled,  it still distinguishes XHTML-MP document and HTML documents, and uses different parsers for different documents.  The documents will be distinguished by MIME type /file extension.  You are right that we should only use XHTML-MP logic when we know that the document IS a XHTML-MP  document. 

The problem is,  when the author of XHTML-MP introduced this feature to WebKit, it causes regressions to HTML document by disabling some code of HTMLParser with :

#if !ENABLE(XHTMLMP)
  original HTMLParser  code for <noscript> processing
#endif

That causes problems with <noscript> processing for HTML documents -- contents inside <noscript> are always parsed and processed no matter Javascript is enabled or not, which is incorrect for HTML document. 

That's my patch tries to fix -- to make contents inside <noscript> ignored when Javascript is enabled for HTML document. 

Hope this makes it clear .

Thanks
Comment 12 Maciej Stachowiak 2009-12-28 01:56:09 PST
Comment on attachment 45415 [details]
another patch

Tests look correct and I agree the #ifdef is bogus. r=me
Comment 13 WebKit Commit Bot 2009-12-28 22:41:36 PST
Comment on attachment 45415 [details]
another patch

Clearing flags on attachment: 45415

Committed r52609: <http://trac.webkit.org/changeset/52609>
Comment 14 WebKit Commit Bot 2009-12-28 22:41:41 PST
All reviewed patches have been landed.  Closing bug.