723463 - execCommand("styleWithCSS", false, null) is not equivalent to execCommand("styleWithCSS", false, false)

Status: UNCONFIRMED

(Core :: DOM: Editor, defect, P5)

Description

[Mattias Lindholm]

Attachment: editableCSS.html

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0.1) Gecko/20100101 Firefox/9.0.1
Build ID: 20111220165912

Steps to reproduce:

Created a document with the JS calls document.execCommand('styleWithCSS',false,null); and document.execCommand('useCSS',false,null);


Actual results:

The styleWithCSS call has no effect on wether CSS will be used or not, whilst useCSS does.


Expected results:

The editable content should use e g <b> rather than <span style="font-weight:bold;"> markup with the styleWithCSS call, just as the deprecated useCSS one does.

Comment 1

[Nickolay_Ponomarev]

It wasn't immediately obvious from the report that the testcase runs *two* execCommand's - first the styleWithCSS or useCSS call you mention in comment 0 (depending on which link you click), then the 'bold' command.

The actual steps to reproduce are:
1. Open testcase
2. Select a word, e.g. "Editable"
3. Click styleWithCSS
4. Inspect the DOM to see the <span style=...> inserted.

The reason it works this way is that we only treat the string "false" as a falsy value for commands requiring a boolean parameter, so the |null| you pass is actually "true". For styleWithCSS "true" means "style with CSS", while for useCSS the meaning of the parameter is inverted, hence the difference in behavior.

Here's the relevant code:
http://hg.mozilla.org/mozilla-central/annotate/bbe5086163c9/content/html/document/src/nsHTMLDocument.cpp#l2885

I'm not sure what we're supposed to do here, so keeping this open for now.

Comment 2

[Boris Zbarsky [:bzbarsky]]

The IDL for execCommand in HTML5 is:

  boolean execCommand(DOMString commandId, boolean showUI, DOMString value);

Since this doesn't have [TreatNullAs=EmptyString], null is converted into the string "null".

What happens after that is defined by http://dvcs.w3.org/hg/editing/raw-file/tip/editing.html#the-usecss-command which says:

  If value is an ASCII case-insensitive match for the string "false", set the CSS styling
  flag to true. Otherwise, set the CSS styling flag to false.

which is what we do (and it's done this way for backwards compat).  For comparison, http://dvcs.w3.org/hg/editing/raw-file/tip/editing.html#the-stylewithcss-command says:

  If value is an ASCII case-insensitive match for the string "false", set the CSS styling
  flag to false. Otherwise, set the CSS styling flag to true.

so passing "null" sets the flag to true.

I agree the behavior is nuts, but it's been nuts of a very long time and web pages depend on that as far as I can tell.

Comment 3

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

I just specced what Gecko did here.  In my testing, WebKit treated the literal case-sensitive string "true" as true, the literal case-sensitive string "false" as false, and ignored the command for any other input -- so it would ignore execCommand("stylewithcss", false, null).

Behavior here is indeed nuts.  I'd be fine with changing the spec to special-case "null" and "" and maybe "undefined" as well, if Gecko and WebKit are willing to change, but that might not be web-compatible.

Comment 4

[(no longer active)]

Yeah I'm not sure how feasible it would be to change the behavior here from the web-compat perspective...  Maybe we should add an error console warning for anything passed to this function except for "true" and "false"?  That way web developers would hopefully start noticing this and would fix their code.

Comment 5

[Jens Stutte [:jstutte]]

Bulk-downgrade of unassigned, >=5 years untouched DOM/Storage bugs' priority.

If you have reason to believe this is wrong (especially for the severity), please write a comment and ni :jstutte.