Bug 113264

Summary: Rename HTMLInputElement::isIndeterminate to Element::shouldAppearIndeterminate
Product: WebKit Reporter: Kent Tamura <tkent>
Component: CSSAssignee: Kent Tamura <tkent>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, allan.jensen, apinheiro, cfleizach, cmarcelo, dmazzoni, eric, esprehn+autocc, haraken, jdiggs, macpherson, menard, mifenton, morrita, ojan.autocc, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Kent Tamura 2013-03-25 19:15:04 PDT 
HTMLInputElement has indeterminate() and isIndeterminate().  It's super confusing.
Comment 1 Kent Tamura 2013-03-25 19:25:35 PDT 
Created attachment 194981 [details]
Patch
Comment 2 Kentaro Hara 2013-03-25 22:32:18 PDT 
Comment on attachment 194981 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=194981&action=review

> Source/WebCore/html/HTMLProgressElement.cpp:173
> +    return !isDeterminate();

As far as I see the removed code in SelectorChecker.cpp, it looks like that isDeterminate() is a negation of isIndeterminate(). In this patch, isIndeterminate() is remaned to shouldAppearIndeterminate(). Then, don't you want to rename isDeterminate() to something consistent as well (e.g. shouldAppearDeterminate()) ?
Comment 3 Kent Tamura 2013-03-25 22:54:59 PDT 
Comment on attachment 194981 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=194981&action=review

>> Source/WebCore/html/HTMLProgressElement.cpp:173
>> +    return !isDeterminate();
> 
> As far as I see the removed code in SelectorChecker.cpp, it looks like that isDeterminate() is a negation of isIndeterminate(). In this patch, isIndeterminate() is remaned to shouldAppearIndeterminate(). Then, don't you want to rename isDeterminate() to something consistent as well (e.g. shouldAppearDeterminate()) ?

I understand you think it's inconsistent, however I feel it's not inconsistent because their roles are different.  shouldAppearDeterminate is a hook for CSS selector matching, isDeterminate represents "determinate" concept in the standard and it is just for HTMLProgressElement.
Comment 4 Kentaro Hara 2013-03-25 23:24:11 PDT 
Comment on attachment 194981 [details]
Patch

Makes sense. Thanks for the clarification.
Comment 5 WebKit Review Bot 2013-03-26 01:28:34 PDT 
Comment on attachment 194981 [details]
Patch

Clearing flags on attachment: 194981

Committed r146860: <http://trac.webkit.org/changeset/146860>
Comment 6 WebKit Review Bot 2013-03-26 01:28:38 PDT 
All reviewed patches have been landed.  Closing bug.