Closed Bug 665027 Opened 13 years ago Closed 13 years ago
Remove useless code which only purpose is to make me feel pain
So, as far as I understand it, .mValue is never used (but I want to use it for <input type=number> so we should keep it) and HandlyTypeChange is already cleaning mInputData so .mValue will always be null. Asking a feedback from Ehsan given that he is the last dev who touched this code and asking the review to Boris hoping he might know more about that chunk. FWIW, the patch has been sent to try and it's green for the moment.
Comment on attachment 540075 [details] [diff] [review] Patch v1 I have no idea what mInputData.mValue is... But yes, HandleTypeChange looks like it sets @value as needed already.
Attachment #540075 - Flags: review?(bzbarsky) → review+
Comment on attachment 540075 [details] [diff] [review] Patch v1 Do we know that mInputData.mValue && !aValue is never true? Because if it is, this code does have a real purpose!
(In reply to comment #2) > Comment on attachment 540075 [details] [diff] [review] [review] > Patch v1 > > Do we know that mInputData.mValue && !aValue is never true? Because if it > is, this code does have a real purpose! I do not understand why you are mentioning |aValue| here. If it is null, we call HandleTypeChange in AfterSetAttr, otherwise it's called in ParseAttribute. Whatever happens, HandleTypeChange will always be called before the code I'm removing. In addition, supposing mInputData.mValue isn't null, it will unlikely be a pointer to a string. mInputData.mState is set but mInputData.mValue, never. IMO, this might have been used in the past and is not anymore and this code is, I think, a relic of that past. I don't want to remove mValue because I'm going to use it for <input type=number> but I want to remove this code... And the try push is fully green.
Whiteboard: [needs review] → [needs feedback]
Ah, yes, you're right.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [needs feedback]
Target Milestone: --- → mozilla7
You need to log in before you can comment on or make changes to this bug.