Open Bug 723463 Opened 9 years ago Updated 9 years ago

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

Categories

(Core :: DOM: Editor, defect)

defect
Not set
normal

Tracking

()

UNCONFIRMED

People

(Reporter: mattias, Unassigned)

Details

(Keywords: testcase, Whiteboard: [needs a spec])

Attachments

(1 file)

Attached file 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.
Attachment #593786 - Attachment mime type: text/plain → text/html
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.
Component: Untriaged → Editor
OS: Windows 7 → All
Product: Firefox → Core
QA Contact: untriaged → editor
Hardware: x86_64 → All
Summary: styleWithCSS has no effect, useCSS (deprecated) does → execCommand("styleWithCSS", false, null) is not equivalent to execCommand("styleWithCSS", false, false)
Version: 9 Branch → Trunk
Keywords: testcase
Whiteboard: [needs a spec]
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.
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.
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.
You need to log in before you can comment on or make changes to this bug.