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

RESOLVED FIXED in mozilla7

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mounir, Assigned: mounir)

Tracking

Trunk
mozilla7
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
Created attachment 540075 [details] [diff] [review]
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.
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!
(Assignee)

Comment 3

6 years ago
(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+
(Assignee)

Comment 5

6 years ago
Pushed:
http://hg.mozilla.org/mozilla-central/rev/2c1c975dd6db
Status: ASSIGNED → RESOLVED
Last Resolved: 6 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.