Bug 48479 – queryCommandValue should fall back to queryCommandState

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Ryosuke Niwa]

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

[Ryosuke Niwa]

Created attachment 72113 [details]
fixes the bug

[Darin Adler]

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.

[Ryosuke Niwa]

(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?

[Ryosuke Niwa]

(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.

[Ryosuke Niwa]

Committed r70810: <http://trac.webkit.org/changeset/70810>

[Aryeh Gregor]

For anyone interested, this is currently being discussed:

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