Closed Bug 142965 Opened 22 years ago Closed 19 years ago

Elements moved out of tables lose their attributes when they're not valid children of tables

Categories

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

x86
Windows 2000
defect

Tracking

()

RESOLVED FIXED
mozilla1.4alpha

People

(Reporter: bugzilla2, Assigned: mrbkap)

References

()

Details

Attachments

(3 files, 1 obsolete file)

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0.0+)
Gecko/20020505
BuildID:    2002050508

The page contains a form with a POST method.
When submitted the browser actually sends a GET which of course the script is
not expecting and basically the script is run with no input

Reproducible: Always
Steps to Reproduce:
1. Go to www.edcint.co.nz/1
2. Click the "delete" button (by the way, you just executed a form which used a
POST that actually worked ok)
3. on the next page press the submit button (it doesn't matter what you put in
because the script never gets it) - this page has a form with a post method
4. you get the page that the script outputs when no input was given

Actual Results:  cgi is run with no input
in this case the script lives and outputs a message

Expected Results:  should have returned some other script generated page

this works in IE
I will attach a packet trace which shows the IP packet that mozilla sends after
you press the submit button
I coulc modify the cgi perl script to get more info if needed
The screwed-up markup causes us to not put the form controls inside the <form>
in the content model, so submission happens to the current URL (action="") with
a default action (GET).  On the site in question the current url and action url
were the same, hiding this problem...

To parser, ccing jkeiser (I suspect that killing DemoteForm will magically fix
this bug).
Assignee: sgehani → harishd
Status: UNCONFIRMED → NEW
Component: XP Apps → Parser
Ever confirmed: true
QA Contact: paw → moied
If it does, that would extreme magic.  We are seriously messing up the DOM in
this example.  It doesn't even look like it has anything to do with DemoteForm()
since in the DOM the input is not demoted to be a sibling of the table.

The resulting content model here is
<p></p>
<h3></h3>
<input/>
<table>
  <form></form>
</table>

And it appears we don't put the attributes onto the form at all.
Unfortunately, DemoteForm removal would not do the trick :(. This is something
that happens at the parser end when handling misplaced table content ( yet
another content juggler! ). 
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla1.0.1
I have made a more minimal testcase.
<html><body>
<table>
<p><h3>Test</h3></p>
<form method="POST">
<input type=submit>
</form>
</table>
</body></html>


using DOM Inspector, I see that form's attribute is missing.

so I change to this testcase.
<html><body>
<table>
<p><h3>Test</h3></p>
<form method="POST">
<input type=submit>
</form>
</table>
</body></html>
I can see form's attribute, but the DOM model is still something wrong.

So this bug must be related with handle table token.


Target Milestone: mozilla1.0.1 → mozilla1.1beta
This is worse than I imagined ( parser just discards the misplaced form
attribtues! ). Will propose a fix soon.
Attached patch patch v1.0 (obsolete) — Splinter Review
Since FORM is not a legal child of TABLE do not handle misplaced table tags, in
the misplaced list, on encountering a FORM.

Problem: Because we started handling misplaced table tags on encountering a
FORM we never got the chance to collect FORM attributes from the tokenizer
stack.
Whiteboard: [fix in hand]
Hmmmm, thinking about it I'm not completely convinced that this is "THE" fix.
Will investigate more.
Did some more testing and found out that the patch does fix few other problems,
such as:

<html>
<body>
 <table border="1" width="100px">
  <tr>
   <p><caption>caption
   <td>
    cell
   </td>
  </tr>
 </table>
</body>
</html>

So, I'm going to stick with the proposed fix.

FYI: Parser regression test PASSED.
Target Milestone: mozilla1.1beta → mozilla1.2beta
Target Milestone: mozilla1.2beta → mozilla1.3alpha
Attachment #90666 - Flags: superreview?(jst)
Attachment #90666 - Flags: review?(heikki)
Comment on attachment 90666 [details] [diff] [review]
patch v1.0

sr=jst
Attachment #90666 - Flags: superreview?(jst) → superreview+
This bug not only occurs in Windows 2000 as listed, but also in Mozilla 1.2 for 
Linux.
Attachment #90666 - Flags: review?(heikki) → review+
Fixed on 12/03/2002.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
I have backed out the patch because of bug 183711.

Reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
This fix also seems to have caused crash bug 184043
This patch turned this bug into a monster. Moving bug to 1.3beta for further
investigation.
Target Milestone: mozilla1.3alpha → mozilla1.3beta
Target Milestone: mozilla1.3beta → mozilla1.4alpha
Blocks: 131700
*** Bug 180334 has been marked as a duplicate of this bug. ***
Summary: Form contains POST but Mozilla sends GET request → Elements moved out of tables lose their attributes when they're not valid children of tables
Whiteboard: [fix in hand]
So the real problem here is the construct: <table></p><form attribute="value">

The </p> was getting pushed onto the misplaced stack and getting handled when
we reached the <form>. </p> is special, however, and is supposed to be treated
like <p></p>. The way this was done was to push the </p> and the <p> back on
the tokenizer. However, at this point we'd popped the <form> off of the
tokenizer but not its attributes. So the form's attributes would end up
_behind_ the <p></p>, causing them not to be collected.

This patch makes us handle the <p></p> directly when we're in misplaced
content, since the new token will get lost otherwise.
Assignee: harishd → mrbkap
Attachment #90666 - Attachment is obsolete: true
Status: REOPENED → ASSIGNED
Attachment #183960 - Flags: review?(jst)
Comment on attachment 183960 [details] [diff] [review]
alternative approach

r=jst
Attachment #183960 - Flags: review?(jst) → review+
Comment on attachment 183960 [details] [diff] [review]
alternative approach

Brendan, could you sr?
Attachment #183960 - Flags: superreview?(brendan)
Comment on attachment 183960 [details] [diff] [review]
alternative approach

Looks good to me, and ok for 1.8b3 -- right?  I'm approving optimistically.

/be
Attachment #183960 - Flags: superreview?(brendan)
Attachment #183960 - Flags: superreview+
Attachment #183960 - Flags: approval1.8b3+
Fix checked in (hopefully for good this time!).
Status: ASSIGNED → RESOLVED
Closed: 22 years ago19 years ago
Resolution: --- → FIXED
Depends on: 414764
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: