Closed Bug 1235990 Opened 6 years ago Closed 6 years ago

JS typeof never returns "array"

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(firefox47 fixed)

RESOLVED FIXED
Tracking Status
firefox47 --- fixed

People

(Reporter: shu, Assigned: _AtilA_)

Details

Attachments

(1 file)

I don't really know what component PersistentDataBlock is, sorry.

In my travels I have seen "array" being checked for when using the typeof operator. This is dead code.

b2g/components/test/unit/file_persistentdatablock.js
36:  } else if (typeof data === "array") {

b2g/components/PersistentDataBlock.jsm
68:  } else if (typeof data === "array") {
Flags: needinfo?(lissyx+mozillians)
Flags: needinfo?(jgomez)
good point, I missed it
Fixed!
There are lot of ways to check if the type is an Array and as we don't actually need to be crossbrowser compatible for internal Gecko code, we could use: Array.isArray() (EMACScript 5) as well... anyway, I followed the suggestion to use instanceof.
Assignee: nobody → jgomez
Flags: needinfo?(lissyx+mozillians)
Flags: needinfo?(jgomez)
Attachment #8705177 - Flags: review?(shu)
Comment on attachment 8705177 [details] [diff] [review]
Correct array type checking using instanceof

Review of attachment 8705177 [details] [diff] [review]:
-----------------------------------------------------------------

Just a question. Can we and would it make sense to add some tests that runs this code? Tests that would have caught this issue if present?
Well, there's no specific tests for this function (toHexString). It's only used for logging purposes.
The component is being well tested here: http://mxr.mozilla.org/mozilla-central/source/b2g/components/test/unit/file_persistentdatablock.js
Comment on attachment 8705177 [details] [diff] [review]
Correct array type checking using instanceof

Review of attachment 8705177 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the delay, was on PTO.
Attachment #8705177 - Flags: review?(shu) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/dc0b2ddbdbb4
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.