'Page info' dialog shows form fields belonging to incorrect forms

RESOLVED FIXED

Status

RESOLVED FIXED
15 years ago
14 years ago

People

(Reporter: bps7j, Assigned: db48x)

Tracking

Trunk
x86
Windows XP

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

15 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031112 Firebird/0.7+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031112 Firebird/0.7+

In a page with multiple forms, the page info dialog doesn't show the fields
correctly; when selecting one form with a mouse, it shows fields that belong to
others and doesn't show fields that belong to it.

Reproducible: Always

Steps to Reproduce:
1. Go to https://adwords.google.com/select/
2. Right-click and view source.
3. Find the string <form action="main#a" for reference.
4. Right-click on the browser again and view page info.
5. Select the Forms tab.  You should see three forms.
6. Select the unnamed form whose method is POST.
7. Select the named form 'langform' whose method is GET.

Actual Results:  
After step 6, the lower half of the dialog displays 5 fields: login.userid,
login.password, login, hl, and start.  After step 7, the lower half displays
those fields, plus three additional fields.

Expected Results:  
It should display login.userid, login.password, cmd, login, and hl.  After step
7, it should display only hl, hl, and null.

it seems as though it's considering that a form contains all fields defined
after the opening <form> tag, without regard to the closing tags.  Thus the
first form in the document contains all fields, and each succeeding one contains
fewer and fewer.  This isn't quite right, though -- because the first form
(langform) doesn't report that it contains the submit element named 'null', and
the second form doesn't report that it contains the hidden 'cmd' element.

Comment 1

15 years ago
->Browser/Page Info
Assignee: blake → db48x
Component: General → Page Info
Product: Firebird → Browser
Version: unspecified → Trunk
The problem is at
http://lxr.mozilla.org/seamonkey/source/xpfe/browser/resources/content/pageInfo.js#685
-- the page info code assumes a form's controls are descendants of the <form> in
the DOM.  On invalid pages like this, that's not the case....
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 3

15 years ago
Hmmm.  I was wondering if that was the problem, too (I don't know the source,
other than just reading through what you referenced).  The page is certainly not
valid HTML, but the tags are correctly nested for the first form, at least.  I
haven't looked at the rest.  Here's the first one:

<form action="main" method="get" style="margin-top: 0px" id="langform"
name="langform">
    <input type="hidden" name="cmd" value="Login">
    <tr valign="top">
        <td nowrap>
            <h4 style="margin-bottom: 4px">
                <font color="#666666" size="-1">It's All About Results&#153;</font>
            </h4>
        </td>
        <td align="right" nowrap>
            <font size="-1">Change Language:
                <select name="hl"
onchange="document.forms[&quot;langform&quot;].submit()">
                    <font size="-1"></font>
                    <option value="da"> Danish</option>
                    <!-- etc etc -->
                    <option value="sv"> Swedish</option>
                </select>
                <noscript>
                    <input type="submit" name="null" value="Go" tabindex="5">
                    </noscript> -&nbsp;
                <a href="index.html">Help</a>
            </font>
        </td>
    </tr>
</form>

I don't know if that means anything, but I'd think that a correct DOM tree would
come out of this, and then the descendants would be a safe bet.  Of course this
doesn't help when the DOM is whacked, I am just wondering if this is the whole
problem.  Or is that really what that function is doing?  It seems to be walking
the rest of the tree in document order, not walking the descendants.  Again, I
don't know this source code.

I also noticed that their ugly HTML confused me.  The first form, 'langform,'
should have elements 'cmd', 'hl', and possibly 'null' (the submit element)
depending on how the noscript tag affects things (?).  The second should have
'cmd', 'hl', 'login.userid', 'login.password', and 'login'.  Sorry about the
false reporting there.
(Reporter)

Comment 4

15 years ago
I stand corrected... I have created a small test case that shows this dialog
DOES work right when the page is valid.

http://www.sequent.org/test.html
> I don't know if that means anything, but I'd think that a correct DOM tree
> would come out of this

  <table> <form> <tr> </tr> </form> </table>

produces the following DOM:

  <table> <form /> <tr> </tr> </table>

since <tr> cannot be contained directly in <form>....

The treewalker fu just needs to go and get replaced by a form.elements traversal...
*** Bug 230428 has been marked as a duplicate of this bug. ***
*** Bug 230428 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 8

15 years ago
Created attachment 138993 [details] [diff] [review]
fix

use form.elements, also fix 220833 while I'm here
(Assignee)

Comment 9

15 years ago
Comment on attachment 138993 [details] [diff] [review]
fix

requesting r=/sr=
Attachment #138993 - Flags: superreview?(bz-vacation)
Attachment #138993 - Flags: review?(jag)
Comment on attachment 138993 [details] [diff] [review]
fix

sr=bzbarsky
Attachment #138993 - Flags: superreview?(bz-vacation) → superreview+

Comment 11

15 years ago
Comment on attachment 138993 [details] [diff] [review]
fix

r=jag assuming this has been teste with both valid and invalid pages with
single and multiple forms. form.elements seems so easy and obvious that it
makes me wonder if there's a reason it wasn't used.
Attachment #138993 - Flags: review?(jag) → review+
(Assignee)

Comment 12

15 years ago
It wasn't used because I never thought of it, probably because it is too easy.
(Assignee)

Comment 13

15 years ago
and now that I'm home and have tested this, I'll can say with 100% certainty
that  this does indeed fix the problem (but why wouldn't it?).
(Assignee)

Comment 14

15 years ago
checked in, marking fixed
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
*** Bug 242964 has been marked as a duplicate of this bug. ***
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.