HTML Form Exception when DOM Object doesn't exist AND FIX

VERIFIED FIXED

Status

()

Core
Layout
P2
normal
VERIFIED FIXED
20 years ago
19 years ago

People

(Reporter: mle, Assigned: Eric Pollmann)

Tracking

Trunk
x86
All
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

(Reporter)

Description

20 years ago
Exception crash when handling HTML Form in JavaScript using DOM interface
when the form object referenced does not exist.

The problem was found to be in /layout/html/content/src/nsHTMLFormElement.cpp
in NamedItem.  The fix for this problem is below.  It should be obvious from
the code that the fix is correct.  First I have included the fixed code,
then the original code with comments showing where the fix should be added
followed by the place where NamedItem is called where the exception
occurs with a comment explaining the problem.

FIXED CODE in nsHTMLFormElement.cpp

NS_IMETHODIMP
nsHTMLFormElement::NamedItem(const nsString& aName, nsIDOMElement** aReturn)
{
  // XXX For now we just search our element list. In reality, we'll have
  // to look at all of our children, including images, objects, etc.
  nsIDOMNode *node;
  nsresult result = mControls->NamedItem(aName, &node);

  if ((NS_OK == result) && (nsnull != node)) {
    result = node->QueryInterface(kIDOMElementIID, (void **)aReturn);
    NS_RELEASE(node);
  }
  else {
	*aReturn = nsnull;
  }

  return result;
}


BROKEN ORIGINAL CODE WITH A COMMENT INDICATING WHAT FIX SHOULD BE
MADE

NS_IMETHODIMP
nsHTMLFormElement::NamedItem(const nsString& aName, nsIDOMElement** aReturn)
{
  // XXX For now we just search our element list. In reality, we'll have
  // to look at all of our children, including images, objects, etc.
  nsIDOMNode *node;
  nsresult result = mControls->NamedItem(aName, &node);

  if ((NS_OK == result) && (nsnull != node)) {
    result = node->QueryInterface(kIDOMElementIID, (void **)aReturn);
    NS_RELEASE(node);
  }
//  result may be NS_OK but node returned NamedItem might
//  be null if the named item control was not found.  Thus
//  we could return an NS_OK value but any garbage that was
//  already in aReturn.  Add an else that makes aReturn null if
//  either result is not OK or node is null.

  return result;
}

HERE IS ONE PLACE WHERE WE GOT AN EXCEPTION:

PR_STATIC_CALLBACK(JSBool)
GetHTMLFormElementProperty(JSContext *cx, JSObject *obj, jsval id, jsval *vp)
{
....
      if (NS_OK == b->NamedItem(name, &prop)) {
        NS_RELEASE(b);

// BUG REPORT COMMENT
// prop does not have to be null because NamedItem
// was not returning a null value if no nameditem
// was found

        if (NULL != prop) {
          // get the js object
          if (prop != nsnull) {
            nsIScriptObjectOwner *owner = nsnull;

// BUG REPORT COMMENT
// And, of course, we crash here since we just have
// garbage for the prop object

            if (NS_OK == prop->QueryInterface(kIScriptObjectOwnerIID,
(void**)&owner)) {

Updated

20 years ago
Assignee: kipp → karnaze

Updated

20 years ago
Assignee: karnaze → pollmann
(Assignee)

Updated

20 years ago
Status: NEW → RESOLVED
Last Resolved: 20 years ago
Resolution: --- → FIXED
(Assignee)

Comment 1

20 years ago
Vidur checked in this fix on Dec 2nd.

Updated

20 years ago
QA Contact: 4144

Comment 2

20 years ago
I would like to check this fix in the latest build but not sure what to look at.
Could a simple
html file be provided to help check to see if this problem was addressed ?
(Assignee)

Comment 3

20 years ago
For a test of this bug see:

http://blueviper/forms/nameditem.html

Prior to the Dec 2nd checkin of this fix, this page would cause this browser to
crash.  Now it (correctly) spits out a Javascript error message:

"document.testForm.testCRASH has no properties"

It no longer crashes.

Updated

20 years ago
Status: RESOLVED → VERIFIED

Comment 4

20 years ago
Fixed in Feb 9th Build.
You need to log in before you can comment on or make changes to this bug.