Bug 82644

Summary: InputType attribute changed functions should happen after the attribute change
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: FormsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, ddkilzer, joepeck, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
[PATCH] Proposed Fix none

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.