Summary: | queryCommandValue should fall back to queryCommandState | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||
Component: | HTML Editing | Assignee: | 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
Ryosuke Niwa
2010-10-27 15:53:57 PDT
Created attachment 72113 [details]
fixes the bug
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. (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? (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. Committed r70810: <http://trac.webkit.org/changeset/70810> For anyone interested, this is currently being discussed: http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2011-September/033235.html |