Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#8097 closed defect (fixed)

[patch] [cla] doh._objPropEq() gives wrong result when one, and only one, of its arguments is 'null'.

Reported by: Joseph Scheuhammer Owned by: Adam Peller
Priority: high Milestone: 1.3
Component: TestFramework Version: 1.2.1
Keywords: Cc: Adam Peller, davidb
Blocked By: Blocking:

Description

The problem shows up in both doh.assertEqual() and doh.asserNotEqual() just before calling this._objPropEq(). They first determine if the typeof their arguments is "object" before making the call.

The problem is that (typeof null) evaluates to "object". Thus, for example, if expected is null, and actual is a non-null object, then doh._objPropEq() is called like this:

	...
	this._objPropEq(null, nonNullObject);
	...

Inside doh._objPropEq() are two for(x in ...) loops for examining the equality of expected's vs, actual's properties. But, trying to access any property of a null throws a type error. FF/Firebug reports, "TypeError: expected is null".

However, doh.assertNotEqual(null, nonNullObject), for example, should succeed since actual is, in fact, not null.

There needs to be a check to insure that both arguments to doh._objPropEq() are non-null before looking at their properties.

Attachments (2)

nullExample.html (874 bytes) - added by Joseph Scheuhammer 11 years ago.
8097.patch (600 bytes) - added by Joseph Scheuhammer 11 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 11 years ago by Joseph Scheuhammer

The attached file, "nullExample.html" shows the error using doh.assertNotEqual().

If running in FF3 with Firebug installed, the error shows up in Firebug's console.

If running in IE7, an alert is posted saying, "'null' is null or not an object".

Changed 11 years ago by Joseph Scheuhammer

Attachment: nullExample.html added

comment:2 Changed 11 years ago by Joseph Scheuhammer

The following patch, "8097.patch", is a way fix the problem.

The rationale is to check for null in doh._objPropEq() itself since, already, two public methods of doh call it. The alternative is to check for null before calling _objPropEq(). The likelihood, if anything, is that the popularity of _objPropEq() will increase; better to put the check in one place.

The logic of the patch is:

  • if both expected and actual are null, return 'true' (their "properties", or lack thereof, do match, OR, they are equal objects).
  • if only one of expected or actual is null, return 'false'.
  • otherwise, do the check as before.

Changed 11 years ago by Joseph Scheuhammer

Attachment: 8097.patch added

comment:3 Changed 11 years ago by Joseph Scheuhammer

Summary: doh._objPropEq() gives wrong result when one, and only one, of its arguments is 'null'.[patch] [cla] doh._objPropEq() gives wrong result when one, and only one, of its arguments is 'null'.

comment:4 Changed 11 years ago by Joseph Scheuhammer

Cc: Adam Peller added

comment:5 Changed 11 years ago by davidb

Cc: davidb added

comment:6 Changed 11 years ago by Becky Gibson

Owner: changed from alex to Adam Peller

comment:7 Changed 11 years ago by Becky Gibson

Milestone: tbd1.3

needed for dialog keyboard doh test

comment:8 Changed 11 years ago by Adam Peller

Resolution: fixed
Status: newclosed

comment:9 Changed 11 years ago by bill

Fixed in [15864].

comment:10 Changed 11 years ago by Adam Peller

see also [15865]

comment:11 Changed 11 years ago by Adam Peller

(In [15865]) missing an '='. Refs #8097 !strict

Note: See TracTickets for help on using tickets.