|Summary:||InputType attribute changed functions should happen after the attribute change|
|Product:||WebKit||Reporter:||Joseph Pecoraro <joepeck>|
|Severity:||Normal||CC:||benjamin, ddkilzer, joepeck, tkent, webkit.review.bot|
|Version:||528+ (Nightly build)|
Description Joseph Pecoraro 2012-03-29 12:48:39 PDT
InputType::disabledAttributeChanged and InputType::readonlyAttributeChanged should happen after the attribute has actually changed. This way if the InputType looks at the state, it would correctly get the new value, not the old value.
Comment 1 Joseph Pecoraro 2012-03-29 12:52:10 PDT
Created attachment 134649 [details] [PATCH] Proposed Fix Saw this while looking at: <http://webkit.org/b/82558> Toggling <input type="range"> readonly or disabled state while active breaks all click events It doesn't really make sense to call the changed function until the value has actually changed!
Comment 2 Eric Seidel (no email) 2012-03-29 13:13:06 PDT
Comment on attachment 134649 [details] [PATCH] Proposed Fix OK. How do we observe this?
Comment 3 Joseph Pecoraro 2012-03-29 13:39:28 PDT
(In reply to comment #2) > (From update of attachment 134649 [details]) > OK. How do we observe this? This doesn't change any existing behavior. The only user of these InputType functions doesn't turn around and change the readonly / disabled state of the input. But if someone wanted to do that than this ordering would matter. I had a patch that used these functions and this confused me. To fix the problem we did not go with that patch, but I still wanted to fix this issue.
Comment 4 Joseph Pecoraro 2012-03-29 13:40:30 PDT
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 134649 [details] [details]) > > OK. How do we observe this? > > The only user of these InputType functions doesn't turn around and > change the readonly / disabled state of the input. I typo'd. This should have said, ".. doesn't turn around and check the state..."
Comment 5 Benjamin Poulain 2012-03-29 14:04:03 PDT
Comment on attachment 134649 [details] [PATCH] Proposed Fix This looks correct to me. I see why you cannot test this, no InputType rely on the attribute value in response to disabledAttributeChanged() or readonlyAttributeChanged().
Comment 6 WebKit Review Bot 2012-03-29 15:32:57 PDT
Comment on attachment 134649 [details] [PATCH] Proposed Fix Clearing flags on attachment: 134649 Committed r112589: <http://trac.webkit.org/changeset/112589>
Comment 7 WebKit Review Bot 2012-03-29 15:33:01 PDT
All reviewed patches have been landed. Closing bug.