Bug 112993

Summary: Implement isEnabledFormControl() for SliderThumbElement and SpinButtonElement in terms of disabled()
Product: WebKit Reporter: Matt Falkenhagen <falken>
Component: Layout and RenderingAssignee: Matt Falkenhagen <falken>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, esprehn+autocc, mifenton, ojan.autocc, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 112085    
Attachments:
Description Flags
Patch none

Matt Falkenhagen
Reported 2013-03-21 19:29:08 PDT
SliderThumbElement and SpinButtonElement's isEnabledFormControl are the only ones that do not just return !disabled(). If we can make them return !disabled(), it would prove that having both disabled and isEnabledFormControl is redundant, and we can just refactor them into a single function.
Attachments
Patch (4.06 KB, patch)
2013-03-21 19:40 PDT, Matt Falkenhagen 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Matt Falkenhagen
Comment 1 2013-03-21 19:40:53 PDT
Created attachment 194420 [details] Patch
Kent Tamura
Comment 2 2013-03-21 19:41:53 PDT
Comment on attachment 194420 [details] Patch ok
WebKit Review Bot
Comment 3 2013-03-22 12:03:49 PDT
Comment on attachment 194420 [details] Patch Clearing flags on attachment: 194420 Committed r146638: <http://trac.webkit.org/changeset/146638>
WebKit Review Bot
Comment 4 2013-03-22 12:03:54 PDT
All reviewed patches have been landed. Closing bug.
Darin Adler
Comment 5 2013-03-22 18:21:33 PDT
Isn’t disabled is a web-exposed property? If so, then it seems that this change is a web-exposed change in behavior, not just a refactoring. If that is right, then we need a regression test and we need to be sure this is a correct change.
Kent Tamura
Comment 6 2013-03-22 18:25:56 PDT
(In reply to comment #5) > Isn’t disabled is a web-exposed property? If so, then it seems that this change is a web-exposed change in behavior, not just a refactoring. If that is right, then we need a regression test and we need to be sure this is a correct change. No. Web-exposed 'disabled' IDL attributes are [Reflected]. Also, SliderThumbElement and SpinButtonElement are not Web-exposed.
Darin Adler
Comment 7 2013-03-22 18:29:30 PDT
(In reply to comment #6) > Web-exposed 'disabled' IDL attributes are [Reflected]. Aha! Then it is critical to eliminate or rename the C++ member function named disabled since it is not the same as the web-exposed function named disabled, which is super-confusing!
Kent Tamura
Comment 8 2013-03-22 18:32:10 PDT
(In reply to comment #7) > Then it is critical to eliminate or rename the C++ member function named disabled since it is not the same as the web-exposed function named disabled, which is super-confusing! Indeed. I'd like to rename HTMLFormControlElement::readOnly too because of the same reason.
Matt Falkenhagen
Comment 9 2013-03-22 18:36:06 PDT
(In reply to comment #7) > Then it is critical to eliminate or rename the C++ member function named disabled since it is not the same as the web-exposed function named disabled, which is super-confusing! I'm hoping to do this: bug 112085, comment 13
Note You need to log in before you can comment on or make changes to this bug.