354703 - calDateTime::SubtractDate has unused variables and function calls

Status: RESOLVED FIXED

(Calendar :: Internal Components, defect)

Description

[Dan Mosedale (:dmosedale, :dmose)]

Which means that the returned duration's nativeTime is wrong (outdated).  I suspect we may want to take this for 0.3, but I'm conflicted.  Taking it scares me, but not taking it scares me too.

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: normalize and cleanup in subtractDate

Comment 2

[cmtalbert]

Comment on attachment 240498 [details] [diff] [review]
normalize and cleanup in subtractDate

This looks good. If there isn't a defect to remove spurious normalize's throughout the rest of the code, can you file one? Or should we just clean them up as we find them?

Comment 3

[Michiel van Leeuwen (email: mvl+moz@)]

I don't understand the bug. Do you have a testcase?
> Which means that the returned duration's nativeTime is wrong (outdated)
calIDuration doesn't have a nativeTime, so how can it be wrong?

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 240498 [details] [diff] [review]
normalize and cleanup in subtractDate

mvl is right; the Normalize call is not necessary here.

Comment 5

[Matthew (lilmatt) Willis]

(In reply to comment #4)
> (From update of attachment 240498 [details] [diff] [review] [edit])
> mvl is right; the Normalize call is not necessary here.    

It looks as if we only need to take the first part of the patch then to fix/close this bug.

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

lilmatt: your commment 5 still applies.  A new version of this patch without the Normalize call is what's wanted here.

Comment 7

[Daniel Boelzle [:dbo]]

BTW: I recognized that calDateTime's boolean member mIsValid is only written but never read, so maybe this can be cleaned up in this turn, too.

Comment 8

[Daniel Boelzle [:dbo]]

(In reply to comment #7)
Forget my previous comment: there is a readonly attribute that reads; I just misgrepped calDatetime.cpp searching for mIsValid while the getter macro just references IsValid...

Comment 9

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: hunk 1 of previous patch

Since this is a quick one, patch as requested in c6.

Comment 10

[Matthew (lilmatt) Willis]

Comment on attachment 260840 [details] [diff] [review]
hunk 1 of previous patch

over to clint

Comment 11

[cmtalbert]

Comment on attachment 260840 [details] [diff] [review]
hunk 1 of previous patch

r=ctalbert

Comment 12

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Checked in on HEAD and MOZILLA_1_8_BRANCH

-> FIXED