Bug 48479

Summary: queryCommandValue should fall back to queryCommandState
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: HTML EditingAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: ayg, darin, enrica, jparent, ojan, rolandsteiner, tkent, tony
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
fixes the bug darin: review+

Description Ryosuke Niwa 2010-10-27 15:53:57 PDT
queryCommandValue should return 'true' or 'false' when the command doesn't return any string and queryCommandState is supported.  This is consistent with IE and will greatly improve WebKit's score on http://www.browserscope.org/richtext2/test
Comment 1 Ryosuke Niwa 2010-10-27 16:54:11 PDT
Created attachment 72113 [details]
fixes the bug
Comment 2 Darin Adler 2010-10-27 17:19:29 PDT
Comment on attachment 72113 [details]
fixes the bug

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

I’m surprised that this functions works that way, but change looks OK. Do any of the test cases cover the mapping of mixed state to "false"?

> LayoutTests/editing/execCommand/script-tests/query-text-alignment.js:7
> +function isEquivalentBoolean(string, bool) {

The function name here isn't all that clear.
Comment 3 Ryosuke Niwa 2010-10-27 17:59:36 PDT
(In reply to comment #2)
> I’m surprised that this functions works that way, but change looks OK. Do any of the test cases cover the mapping of mixed state to "false"?

I'm pretty certain that query-command-state.js do test that.

> > LayoutTests/editing/execCommand/script-tests/query-text-alignment.js:7
> > +function isEquivalentBoolean(string, bool) {
> 
> The function name here isn't all that clear.

I don't like that name either but couldn't come up with a better name.  isEquivalentBooleanValue?  isBooleanValueEqualTo?  isBooleanEqualTo?
Comment 4 Ryosuke Niwa 2010-10-28 14:20:50 PDT
(In reply to comment #3)
> > > LayoutTests/editing/execCommand/script-tests/query-text-alignment.js:7
> > > +function isEquivalentBoolean(string, bool) {
> > 
> > The function name here isn't all that clear.
> 
> I don't like that name either but couldn't come up with a better name.  isEquivalentBooleanValue?  isBooleanValueEqualTo?  isBooleanEqualTo?

Tony pointed out that we can just do centerValue != center.toString() and get rid of the function.  I'll land after making this change.
Comment 5 Ryosuke Niwa 2010-10-28 14:39:00 PDT
Committed r70810: <http://trac.webkit.org/changeset/70810>
Comment 6 Aryeh Gregor 2011-09-16 11:41:21 PDT
For anyone interested, this is currently being discussed:

http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2011-September/033235.html