Last Comment Bug 665027 - Remove useless code which only purpose is to make me feel pain
: Remove useless code which only purpose is to make me feel pain
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla7
Assigned To: Mounir Lamouri (:mounir)
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-17 10:03 PDT by Mounir Lamouri (:mounir)
Modified: 2011-06-20 04:50 PDT (History)
2 users (show)
mounir: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (1.80 KB, patch)
2011-06-17 10:03 PDT, Mounir Lamouri (:mounir)
bzbarsky: review+
ehsan: feedback+
Details | Diff | Splinter Review

Description Mounir Lamouri (:mounir) 2011-06-17 10:03:01 PDT
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.
Comment 1 Boris Zbarsky [:bz] (still a bit busy) 2011-06-17 11:46:44 PDT
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 :Ehsan Akhgari 2011-06-17 12:35:34 PDT
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) 2011-06-18 11:46:26 PDT
(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 :Ehsan Akhgari 2011-06-19 13:20:09 PDT
Ah, yes, you're right.
Comment 5 Mounir Lamouri (:mounir) 2011-06-20 04:50:55 PDT
Pushed:
http://hg.mozilla.org/mozilla-central/rev/2c1c975dd6db

Note You need to log in before you can comment on or make changes to this bug.