Closed Bug 178258 Opened 22 years ago Closed 14 years ago

HTML parser ships <script> to implicit <head>, breaking document.forms or document.getElementById

Categories

(Core :: DOM: HTML Parser, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9.3a5

People

(Reporter: ggeens, Unassigned)

References

()

Details

(Keywords: html5, testcase, Whiteboard: [fixed by the HTML5 parser])

Attachments

(2 files, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.1) Gecko/20020826
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.1) Gecko/20020826

In the checkout process, the site uses a short "forwarding" page. The page
contains a hidden form and some Javascript to relay the user to the next step.

This mechanism works on IE, but fails on Mozilla (1.0 and 1.1).

Reproducible: Always

Steps to Reproduce:
1. Go to http://www.azur.be/
2. Select one or more articles from the catalog
3. Go to the checkout page. This will prompt you to create a profile.
4. Select a payment type.
Actual Results:  
Mozilla shows a blank page. The Javascript console contains the error:
document.form1 has no properties. (The page source is included below.)

Expected Results:  
Submit the form and continue with the next step in the checkout process.

Page source:
<form action="/secure/order/order_purchase_overschr.asp" method=post id=form1
name=form1>
	
	<input type=hidden name="kortingo" value="">
	<input type=hidden name="korting" value="">
	<input type=hidden name="return_shop" value="">
	<input type=hidden name="preOrderReduceNote" value="">
	<input type=hidden name="betwijze" value="over">
	<input type=hidden name="VoucherCode" value="">
</form>

<script language="Javascript">
	document.form1.submit();
</SCRIPT>
The site has closed down. It doesn't make sense to keep it as a Tech Evangelism bug.

I'm reassigning this to Browser: the page from my original bug report should
post the form, but it doesn't. (This is different from the document.all
construct IMHO.)
Component: Europe: West → JavaScript Engine
Product: Tech Evangelism → Browser
Version: unspecified → 1.0 Branch
Browser, not engine ---> Parser for further triage.

Using Mozilla trunk binary 20021204xx on WinNT. I'll attach
two reduced testcases below, showing why we get the error

            "document.form1 has no properties"


------------------------- Testcase #1 -------------------------
<form id=form1 name=form1></form>

<script>
  alert('document.form1 is: ' + document.form1);
</SCRIPT>
---------------------------------------------------------------

------------------------- Testcase #2 -------------------------
<body>
<form id=form1 name=form1></form>
</body>

<script>
  alert('document.form1 is: ' + document.form1);
</SCRIPT>
---------------------------------------------------------------

Note the only difference is that testcase #2 has a body element,
while testcase #1 doesn't.


--------------------------- OUTPUT ----------------------------

Mozilla: testcase #1 produces 'undefined'
         testcase #2 produces '[object HTMLFormElement]'

IE6: both testcases produced: '[object]'
NN4: both testcases produced: '[object Form]'


So it seems that Mozilla needs the form to be inside the body element
in order to recognize it. I leave it to the Parser folks to comment
on whether this is a bug or not. Certainly it is not backward-compatible
with NN4.
Assignee: nitot → harishd
Status: UNCONFIRMED → NEW
Component: JavaScript Engine → Parser
Ever confirmed: true
QA Contact: brantgurganus2001 → moied
Changing to All and Trunk.
Old summary:
Javascript error: document.form1 has no properties
New summary:
document.forms has no properties on a page without <body> (JavaScript error)

Related bugs are bug 144780 and bug 185438.
OS: Windows 2000 → All
Hardware: PC → All
Summary: Javascript error: document.form1 has no properties → document.forms has no properties on a page without <body> (JavaScript error)
Version: 1.0 Branch → Trunk
*** Bug 185438 has been marked as a duplicate of this bug. ***
*** Bug 178773 has been marked as a duplicate of this bug. ***
*** Bug 224619 has been marked as a duplicate of this bug. ***
*** Bug 227798 has been marked as a duplicate of this bug. ***
Keywords: testcase
*** Bug 153542 has been marked as a duplicate of this bug. ***
From bug 153542:

It looks like the parser has explicit code to handle the following case:

  If we have not seen a <body> tag and we see a <script> tag, move that <script>
  tag over under the <head> tag.

In the process, the <script> is actually appended to the document _before_ the
<form> is, which causes all sorts of fun stuff to happen.  See
CNavDTD::HandleStartToken

I suspect that code is there for a reason, and that removing or changing it
should not be undertaken lightly...
*** Bug 229164 has been marked as a duplicate of this bug. ***
*** Bug 181526 has been marked as a duplicate of this bug. ***
*** Bug 233745 has been marked as a duplicate of this bug. ***
Some of these testcase work in IE, but others do the same in IE as Mozilla. 
Opera seems to reliably put the elements in the document in the order they are 
found, but that might be a backcompat bug.
Assignee: harishd → parser
Could you tell me which of these work in IE and on which IE acts like Mozilla?
Browsers tested:
   Opera 7.50 internal pre-release build on Windows 2000.
   Mozilla Firefox 0.8 official build on Windows 2000.
   Internet Explorer 6.1 preview release on Windows 2000.

http://bugzilla.mozilla.org/attachment.cgi?id=107176&action=view
Objects accessible in all three browsers.

http://off.net/~shaver/gebi.html
Object accessible in Opera, null in IE and Mozilla.

http://bugzilla.mozilla.org/attachment.cgi?id=109480&action=view
Object accessible in Opera and IE, undefined in Mozilla.

http://bugzilla.mozilla.org/attachment.cgi?id=88761&action=view
Opera and IE show "test1" then "test2". Mozilla first shows "has no properties" 
and then "test2".

http://bugzilla.mozilla.org/attachment.cgi?id=88763&action=view
All three show "test1" followed by "test2".

http://bugzilla.mozilla.org/attachment.cgi?id=105403&action=view
Accessible in all three browsers.

http://66.54.199.11/~netchess/bug.html
Shows "Testing" in Opera and IE, nothing in Mozilla.

http://bugzilla.mozilla.org/attachment.cgi?id=109344&action=view
Object accessible in Opera and IE, undefined in Mozilla.

HTH.
*** Bug 211489 has been marked as a duplicate of this bug. ***
Blocks: 239182
OK, so it looks like <div> does not prevent the <script> from moving into <head>
in IE, but <form> does.  Any other such magic HTML tags that keep IE from moving
the <script> into the head if they appear before the <script> (and do they need
to contain the <script>, or can they be closed before the <script>?)
Keywords: qawanted
QA Contact: moied → ian
Flags: blocking1.8a4?
Flags: blocking-aviary1.0?
We've got a reduced testcase and quite a few dupes. Who can look into this?
Flags: blocking1.8a4? → blocking1.8a4+
We need some comprehensive testcases to answer the question in comment 19. 
Sounds like a good QA exercise to me... we have some of those around, right?
Flags: blocking1.8a4+
if we get close to a safe patch renominate.  thanks...
Flags: blocking-aviary1.0? → blocking-aviary1.0-
Strange enough document.forms HAS properties if the form contains an <input
type=text> or <input type=submit> although there is no <body> tag!

Works, as there is <body>:

<html>
<body>
<form name="form"><select name="sel"><option></select></form>
<script>alert(document.form.sel.options.length);</script>
</body>
</html>

Does not work (Error: document.form has no properties):

<html>
<form name="form"><select name="sel"><option></select></form>
<script>alert(document.form.sel.options.length);</script>
</html>

And this again DOES work, also with a input type=text, WITHOUT <body>:

<html>
<form name="form"><select name="sel"><option></select><input type=submit></form>
<script>alert(document.form.sel.options.length);</script>
</html>
This bug breaks the freeipods.com signup process.
*** Bug 300644 has been marked as a duplicate of this bug. ***
*** Bug 324262 has been marked as a duplicate of this bug. ***
*** Bug 309469 has been marked as a duplicate of this bug. ***
*** Bug 343808 has been marked as a duplicate of this bug. ***
*** Bug 363899 has been marked as a duplicate of this bug. ***
> Sounds like a good QA exercise to me... we have some of those around, right?

bz, please remember that you *did* ask, and the whole don't kill the messenger thing.

Assuming shaver's gebi.html is what you wanted tested, and that http://www.w3.org/TR/html4/index/elements.html is a complete list of elements, in IE6, with an empty <element id="myid"></element>:

Return [Object]:
abbr, applet, area, base, basefont, body, br, button, caption, col, colgroup, dd, dt, form, frame, head, hr, html, iframe, img, input, isindex, legend, li, link, map, meta, noframes, noscript, object, optgroup, option, param, pre, script, select, style, table, tbody, td, textarea, tfoot, th, thead, title, tr

Return null:
a, acronym, address, b, bdo, big, blockquote, center, cite, code, del, dfn, dir, div, dl, em, fieldset, font, h1, h2, h3, h4, h5, h6, i, ins, kbd, label, menu, ol, p, q, s, samp, small, span, strike, strong, sub, sup, tt, u, ul, var

I didn't figure out how to test frameset.

In every case (except frameset), <element id="myid">foo</element> returns [Object], other than the rather odd case of <object id="myid">foo</object> returning null. I assume an object that wasn't rendering fallback content wouldn't, but I didn't test that.

> (and do they need to contain the <script>, or can they be closed before
> the <script>?)

Makes no difference: the results are exactly the same with <element><script></script></element> as they are with <element></element><script></script>.
Note that HTML5 defines that <script> is no longer moved to the <head> in these cases. (Similar to what Opera does. FWIW, it doesn't appear to break pages for us.) 
Hmm.  Maybe we should in fact just stop shipping out scripts to the head...
Summary: document.forms has no properties on a page without <body> (JavaScript error) → HTML parser ships <script> to implicit <head>, breaking document.forms or document.getElementById
Assignee: parser → mrbkap
QA Contact: ian → parser
Nominating because it seems like a win-win situation. The fix would help HTML5, Acid3 and reduce the number of dupes filed.
Flags: wanted1.9.1?
Flags: blocking1.9.1?
Attached patch patch that doesn't work (obsolete) — Splinter Review
I sort of thought this would fix the Acid3 example of this bug, but it doesn't.

At lunch, mrbkap made it sound (at lunch) like it would be more involved than this to fix this bug (by not shipping scripts to the head), although he (just now) seemed a little surprised that this didn't work.
mrbkap pointed out that script doesn't "prefer body", which is why the patch doesn't fix Acid3.

He also pointed out that we should probably look at the list in comment 30 and at what, exactly, HTML5 says to do.
Attached patch Like so? (obsolete) — Splinter Review
This seems to fix it.  It passes mochitests with the exception of
parser/htmlparser/tests/mochitest/test_bug418464.html
which appears to depend on this bug, so I changed the test.

It also passes the parser html regession tests with the exception of
parser/htmlparser/tests/html/149877.html
where we currently move the last <script> into HEAD, but with this patch
it remains in BODY (the first <script> is still moved to HEAD).
I believe the new parsing is correct and it doesn't affect the
end result of the test which still reads "should see this content".

Acid3: I get 86 before, 87 after the patch.
Mats, did you want to ask mrbkap for review?
Flags: wanted1.9.1?
Flags: wanted1.9.1+
Flags: blocking1.9.1?
Flags: blocking1.9.1-
Priority: -- → P2
Any decision or progress on that?
Keywords: html5
Flags: blocking1.9.2?
Depends on: html5-parsing
Any update on the review of the patch?
Update plx
(In reply to comment #39)
> Created an attachment (id=339167) [details]
> Like so?
> 
> This seems to fix it.  It passes mochitests with the exception of
> parser/htmlparser/tests/mochitest/test_bug418464.html
> which appears to depend on this bug, so I changed the test.
> 
> It also passes the parser html regession tests with the exception of
> parser/htmlparser/tests/html/149877.html
> where we currently move the last <script> into HEAD, but with this patch
> it remains in BODY (the first <script> is still moved to HEAD).
> I believe the new parsing is correct and it doesn't affect the
> end result of the test which still reads "should see this content".

This patch only fixes the issue if html5.enable is set to false.
(In reply to comment #45)
> This patch only fixes the issue if html5.enable is set to false.

I filed bug 501345 on the html5 parser issue.
Flags: wanted1.9.2?
Not blocking 1.9.2 on this. Ideally we'd only need to fix this with the HTML5 parser and not in the current parser...
Flags: blocking1.9.2? → blocking1.9.2-
Could we nominate this again for 1.9.3 ?
status2.0: --- → ?
The plan for 1.9.3 is to ship the html5 parser.
Attachment #339167 - Flags: review?(mrbkap)
Blocks: 549490
Assignee: mrbkap → nobody
Now that the HTML5 parser is enabled by default, should this be WONTFIXed?
The HTML5 parser fixed it, didn't it? If so, this should be resolved as fixed.
Note that this depends on the html5 parser tracking bug.  Once it's decided that it has been enabled and sticks, we should probably go through those deps and mark fixed as needed.

Though we're still using our old parser for some things so far; not sure whether it matters for this bug.
(In reply to comment #52)
> The HTML5 parser fixed it, didn't it? If so, this should be resolved as fixed

It would seem you are correct.  Silly me was thinking not fixing in old parser so WONTFIX, but since landing of the HTML5 parser and enabling it by default fixes the bug as described, it would seem that FIXED is the correct resolution here.
HTML5 Parser has been re-enabled, this should be marked as FIXED
Status: NEW → RESOLVED
Closed: 14 years ago
status2.0: ? → ---
Flags: wanted1.9.2?
Keywords: qawanted
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
Attachment #339125 - Attachment is obsolete: true
Attachment #339167 - Attachment is obsolete: true
Whiteboard: [fixed by the HTML5 parser]
You need to log in before you can comment on or make changes to this bug.