Closed Bug 665027 Opened 11 years ago Closed 11 years ago

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

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla7

People

(Reporter: mounir, Assigned: mounir)

Details

Attachments

(1 file)

Attached patch Patch v1Splinter Review
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.
Attachment #540075 - Flags: review?(bzbarsky)
Attachment #540075 - Flags: feedback?(ehsan)
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.
Attachment #540075 - Flags: feedback?(ehsan) → feedback+
Pushed:
http://hg.mozilla.org/mozilla-central/rev/2c1c975dd6db
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Whiteboard: [needs feedback]
Target Milestone: --- → mozilla7
You need to log in before you can comment on or make changes to this bug.