665027 - Remove useless code which only purpose is to make me feel pain

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Mounir Lamouri (:mounir)]

Attachment: Patch v1

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 1

[Boris Zbarsky [:bzbarsky]]

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.

Comment 2

[(no longer active)]

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!

Comment 3

[Mounir Lamouri (:mounir)]

(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.

Comment 4

[(no longer active)]

Ah, yes, you're right.

Comment 5

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/2c1c975dd6db