Closed Bug 142965 Opened 23 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)

Product:
Core
Component:
DOM: HTML Parser
Platform:
x86
Windows 2000
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.4alpha

People

(Reporter: bugzilla2, Assigned: mrbkap)

References

(
URL
)

Details

Attachments

(3 files, 1 obsolete file)

Packet trace of GET when POST was requested
3.22 KB, text/plain
Details
Minimal-ish testcase
284 bytes, text/html
Details
alternative approach
2.34 KB, patch
jst
: review+
brendan
: superreview+
brendan
: approval1.8b3+
Details | Diff | Splinter Review
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 ago → 19 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

Creator:
Created:
Updated:
Size: