Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

Bug 802095 - [Media Storage] TypeError: dsfile.lastModifiedDate is null #5839

Merged
merged 1 commit into from Oct 17, 2012
Merged

Bug 802095 - [Media Storage] TypeError: dsfile.lastModifiedDate is null #5839

merged 1 commit into from Oct 17, 2012

Conversation

ttaubert
Copy link
Contributor

No description provided.

@@ -988,7 +988,8 @@ var MediaDB = (function() {
// 4a: date and size are the same for both: do nothing
// 4b: file has changed: it is both a deletion and a creation
if (dsfile.name === dbfile.name) {
if (dsfile.lastModifiedDate.getTime() !== dbfile.date ||
if (dsfile.lastModifiedDate &&

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't review this without more explanation...

the lastModifiedDate = null is a known device storage bug, and a fix is pending, I think.

If we make the change you propose here, where will we handle these files with no date? Are you suggesting that if a no-date file has the same name and size as a known existing file we treat it as an existing file rather than a new file?

Don't you need parens around teh || expression, though? I think your code doesn't test size if the date is null

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On further thought, I understand this and like it. And I was wrong about the parens. I'd like (but you don't need) parens around the && expression.

Have you checked that there aren't any other similar lastModifiedDate uses in the code?

Thanks for fixing this. r+

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like (but you don't need) parens around the && expression.

I agree, it's much more explicit - will do.

Have you checked that there aren't any other similar lastModifiedDate uses in the code?

No... let me check that right away. Should I file new bugs for these?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there are any, I'd just fix them in this patch. Note that null is probably okay, as long as we don't try to call getTime() on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't find any other usages except one that was already worked around and waits for bug 793955.

@davidflanagan
Copy link

Thanks for the update. r+ to land this.

ttaubert added a commit that referenced this pull request Oct 17, 2012
…lastModifiedDate

Bug 802095 - [Media Storage] TypeError: dsfile.lastModifiedDate is null r=djf a=vingtetun
@ttaubert ttaubert merged commit 2f66c8a into mozilla-b2g:master Oct 17, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants