86964 – Unsupported commands should have queryCommandValue() = "", not false

Bug 86964 - Unsupported commands should have queryCommandValue() = "", not false

Summary: Unsupported commands should have queryCommandValue() = "", not false

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	HTML Editing (show other bugs)
Version:	528+ (Nightly build)
Hardware:	Unspecified Unspecified

Importance:	P2 Normal
Assignee:	Nobody

URL:
Keywords:

Depends on:
Blocks:

Reported:	2012-05-20 07:58 PDT by Aryeh Gregor
Modified:	2012-05-20 20:16 PDT (History)
CC List:	6 users (show)

See Also:

Attachments
ProposedPatch (5.86 KB, patch) 2012-05-20 15:53 PDT, Joe Thomas	rniwa: review+ rniwa: commit-queue-	Details \| Formatted Diff \| Diff
Patch-Updated (6.04 KB, patch) 2012-05-20 17:22 PDT, Joe Thomas	no flags	Details \| Formatted Diff \| Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Aryeh Gregor 2012-05-20 07:58:06 PDT

Test-case:

data:text/html,<!doctype html>
<script>
var val = document.queryCommandValue("quasit");
alert(typeof val + ' "' + val + '"');
</script>

Chrome 20 dev alerts 'boolean "false"'.  This means queryCommandValue() is returning a boolean, when it's specced to always return a string.  Gecko now returns the empty string, per spec <https://bugzilla.mozilla.org/show_bug.cgi?id=742240#c18>.  IE throws, which we decided in bug 83993 was not acceptable.  WebKit should return the empty string here.  This probably doesn't have big compat impact, because false == "" in JavaScript.

Comment 1 Joe Thomas 2012-05-20 14:25:07 PDT

I will create a patch for this

Comment 2 Joe Thomas 2012-05-20 15:53:39 PDT

Created attachment 142923 [details]
ProposedPatch

Comment 3 Ryosuke Niwa 2012-05-20 17:01:55 PDT

Comment on attachment 142923 [details]
ProposedPatch

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

> LayoutTests/editing/execCommand/use-css.html:51
>      testFailed("queryCommandValue('useCSS') should return boolean false");

Please update this comment as well.

Comment 4 Joe Thomas 2012-05-20 17:22:53 PDT

Created attachment 142928 [details]
Patch-Updated

Comment 5 Joe Thomas 2012-05-20 17:23:44 PDT

(In reply to comment #3)
> (From update of attachment 142923 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=142923&action=review
> 
> > LayoutTests/editing/execCommand/use-css.html:51
> >      testFailed("queryCommandValue('useCSS') should return boolean false");
> 
> Please update this comment as well.

Done. I missed it.

Comment 6 WebKit Review Bot 2012-05-20 18:46:33 PDT

Comment on attachment 142928 [details]
Patch-Updated

Clearing flags on attachment: 142928

Committed r117721: <http://trac.webkit.org/changeset/117721>

Comment 7 WebKit Review Bot 2012-05-20 18:46:38 PDT

All reviewed patches have been landed.  Closing bug.

Comment 8 Joe Thomas 2012-05-20 20:16:49 PDT

Thanks for the review :)