Bug 13490 - Implement execCommand("styleWithCSS", ...)
Summary: Implement execCommand("styleWithCSS", ...)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P2 Enhancement
Assignee: Justin Garcia
URL:
Keywords: GoogleBug, InRadar
: 12321 (view as bug list)
Depends on:
Blocks:
 
Reported: 2007-04-25 12:39 PDT by Alexey Proskuryakov
Modified: 2009-02-07 09:30 PST (History)
12 users (show)

See Also:


Attachments
patch (11.40 KB, patch)
2008-02-06 13:22 PST, Justin Garcia
no flags Details | Formatted Diff | Diff
patch (12.59 KB, patch)
2009-02-03 13:18 PST, Justin Garcia
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2007-04-25 12:39:23 PDT
From bug 12198 comment 1.

Consider implementing document.execCommand("styleWithCSS", false, bool) as in
Gecko. Depending on the boolean value, formatting commands should use tags or styled spans.
Comment 1 Danny Bloemendaal 2007-12-21 04:04:20 PST
I think that this support is absolutely vital for webkit to be used in cms environments that expect symantically correct content. Without this, webkit inserts spans instead of <em>s etc. Any postprocessing is impossible in this case to work around this omission.

Here you can see more information about this and other problems:

http://www.quirksmode.org/dom/execCommand.html
Comment 2 Mark Rowe (bdash) 2008-01-12 19:11:31 PST
<rdar://problem/5685604>
Comment 3 Justin Garcia 2008-02-01 12:58:25 PST
*** Bug 12321 has been marked as a duplicate of this bug. ***
Comment 4 Justin Garcia 2008-02-05 14:49:54 PST
(In reply to comment #1)
> I think that this support is absolutely vital for webkit to be used in cms
> environments that expect symantically correct content. Without this, webkit
> inserts spans instead of <em>s etc.

You write that without this feature, Safari inserts spans "instead of <em>s", but both the Quirksmode document that you sited and Mozillas documentation on styleWithCSS say that when styleWithCSS is off, Bold applies a <b> tag.  This is how FF behaves.  Is there a browser that inserts <em>s on Bold?

Justin
Comment 5 Justin Garcia 2008-02-06 13:22:18 PST
Created attachment 18968 [details]
patch
Comment 6 Anders Jenbo 2008-02-09 22:52:19 PST
(In reply to comment #4)
> (In reply to comment #1)
> > I think that this support is absolutely vital for webkit to be used in cms
> > environments that expect symantically correct content. Without this, webkit
> > inserts spans instead of <em>s etc.
> You write that without this feature, Safari inserts spans "instead of <em>s",
> but both the Quirksmode document that you sited and Mozillas documentation on
> styleWithCSS say that when styleWithCSS is off, Bold applies a <b> tag.  This
> is how FF behaves.  Is there a browser that inserts <em>s on Bold?
> Justin

No there is no browser that sees <em> as bold, but italic (<em>), underlined (<u>), strikethrew (<strike>), subscript (<sub>) and supperscript (<sup>) are also affected by this issue.
Comment 7 Anders Jenbo 2008-02-09 23:21:17 PST
(In reply to comment #5)
> Created an attachment (id=18968) [edit]
> patch

I took a look at your patch and I just want to make something clear.
Semantically correct tagging helps Google prioritize a document and allows for better use of external style sheets to easily redesign a page, as well as hint screen readers to alter it's tone, this is why it is so important to use them instead of just using span and style on every thing.

Semantically correct tags are part of HTML 2-5 and XHTML 1-2 so it's not legacy, it's just not css.

<font> tag on the other hand is legacy as they are not semantical and are missing from XHTML and HTML 5.
Comment 8 Adam Strzelecki 2008-02-12 03:07:57 PST
Any chance to have this done before 3.1, currently WebKit (Safari) is useless for CMS editing with WordPress, MODx, etc. just because it inserts Apple-style-span and lot of messy unstylable code.
Comment 9 Justin Garcia 2008-02-13 11:38:39 PST
> Semantically correct tags are part of HTML 2-5 and XHTML 1-2 so it's not
> legacy, it's just not css.

You're right.  I'll be taking another look at this bug soon.  Although, I must say, FF doesn't appear to behave in the way you describe (using <em>, <strong>, etc.).  Am I missing something?
Comment 10 Dan POPA 2008-02-13 13:49:24 PST
(In reply to comment #9)
> > Semantically correct tags are part of HTML 2-5 and XHTML 1-2 so it's not
> > legacy, it's just not css.
> You're right.  I'll be taking another look at this bug soon.  Although, I must
> say, FF doesn't appear to behave in the way you describe (using <em>, <strong>,
> etc.).  Am I missing something?

FF uses B and I, IE uses STRONG and EM, both versions are valid in HTML 4.01 transitional and Strict, in XHTML 1.0 Transitional and Strict and also in HTML 5.
Comment 11 Justin Garcia 2008-02-13 14:30:37 PST
> FF uses B and I, IE uses STRONG and EM, both versions are valid in HTML 4.01
> transitional and Strict, in XHTML 1.0 Transitional and Strict and also in HTML
> 5.

But <b>, <i>, and <font>, which FF uses, are not semantical.  So I'm curious as to why this issue is a showstopper for Safari but not FF.

Justin
Comment 12 Anders Jenbo 2008-02-13 22:32:38 PST
> But <b>, <i>, and <font>, which FF uses, are not semantical.  So I'm curious as
> to why this issue is a showstopper for Safari but not FF.
> Justin

The difference between <b> and <strong>, is that <b> is highlighting with out making the word important.
As it is very hard to know if the users sees a word as "should be highlighted" or "has importance" and both provide roughly the same function and most software renders them the same, both are acceptable. It would be fine if Safari used either, but it uses a span with a style witch is as generic as it can get, and to provide any function the software would have to understand CSS and not just HTML.
That said I am in favor of <strong> as the users is likely to mark a word in bold because it has importance.
Comment 13 Anders Jenbo 2008-02-13 22:53:08 PST
p.s, IE and FF's use of <font> is an issue as it forces every thing they generate to use either XHTML 1.0 transitional or HTML 4 and it has no value over CSS. FF in CSS mode is just as much a show stopper as the current Safari implementation.

Opera has chosen to use <strong> and <i> and also uses <font>.

One benefit of <font> is that the content will be compatible with IE, but I would much rather see IE change there implementation to support CSS styling instead of <font>.
Comment 14 Graham Perrin 2008-09-12 07:37:03 PDT
I am referred to this bug 13490 in connection with 
http://dev.plone.org/plone/ticket/7446 and 
http://dev.plone.org/plone/ticket/8085 

In the context of Kupu, I have a sense that 
<span class="Apple-style-span"> is most problematic. 

Please, how can we progress here? 
Comment 15 Julie Parent 2008-09-12 09:36:18 PDT
Just to chime in with a couple additional reasons why support for this command is important:

* The size of the html generated by contentEditable in Webkit is always much larger than that generated by IE or FF (with styleWithCSS false).  For large documents, there are a lot of bytes consisting of "<span class="Apple-style-span" style="font-weight: bold;">" that could be replaced with "<b>".  These bytes really add up: for applications doing auto-save, this is a lot of extra network bandwidth being used up.  But even more practically, this slows down the initial load of a page that has html generated with Webkit since there are so many more bytes to download and parse.  So in a way, it does lead to a poorer user experience in Webkit based browsers.

* For rich text editing apps that need to work across different browsers (for instance for collaboration), having markup that is this different leads to a number of issues where the other browsers can't undo formatting created in Webkit.
Comment 16 Maciej Stachowiak 2008-11-04 11:19:50 PST
I'd say we should use <b> and <i>, not <strong> and <em>. First of all this matches Firefox. Second, Bold and Italic are presentational commands, and it is appropriate to use presentational markup. If a user hit a button labeled [I] or hit Cmd-I, it would be wrong to infer that he meant to emphasize, since there are many other valid uses of italics. It whoudl be good to get this fix in since we have a patch that looks reasonable.
Comment 17 Anders Jenbo 2008-11-04 15:55:42 PST
Sounds like a good idea.
Comment 18 Danny Bloemendaal 2009-01-28 01:59:26 PST
Is there any progress regarding this issue? It seems that webkit still creates horrible html in combination with content editors.
Comment 19 Justin Garcia 2009-02-03 13:18:51 PST
Created attachment 27285 [details]
patch

updated patch
Comment 20 Darin Adler 2009-02-03 14:02:32 PST
Comment on attachment 27285 [details]
patch

> +    if (value != "false" && value != "true")
> +        return false;

Would be good to have test cases that involve strings like "0" and "1" and "undefined" to check how this compares with other browsers' behavior.

> +    void setStyleWithCSS(bool flag) { m_styleWithCSS = flag; }
> +    bool styleWithCSS() const { return m_styleWithCSS; }

Maybe add "should" to this name?

> +    if (position.node())
> +        if (Document* document = position.node()->document())
> +            m_usesLegacyFormattingTags = !document->frame()->editor()->styleWithCSS();

Should have braces around this.

> -StyleChange::ELegacyHTMLStyles StyleChange::styleModeForParseMode(bool isQuirksMode)
> -{
> -    return isQuirksMode ? UseLegacyHTMLStyles : DoNotUseLegacyHTMLStyles;
> -}

Do we want to preserve the non-quirks mode rule here? If so, then the style with CSS flag should default to true for non-quirks documents. We should at least have a test case for this.

I'm going to say r=me for now, but it seems this is under-tested.
Comment 21 Ojan Vafai 2009-02-03 14:54:50 PST
I don't think it makes sense to tie this to quirks vs standards modes. We can default this to true and match FF behavior or default it to false and match IE behavior (although IE doesn't have this command, so it's effectively always false there).

Either default seems fine to me.
Comment 22 Justin Garcia 2009-02-03 15:28:13 PST
(In reply to comment #20)
> (From update of attachment 27285 [details] [review])
> > +    if (value != "false" && value != "true")
> > +        return false;
> 
> Would be good to have test cases that involve strings like "0" and "1" and
> "undefined" to check how this compares with other browsers' behavior.

FF seems to activate the feature for undefined, null, number values and strings other than "false", as well as the boolean true.  Besides honoring "true" and "false", which we already do, I'm not sure that mimicking that behavior is that useful.


> > +    void setStyleWithCSS(bool flag) { m_styleWithCSS = flag; }
> > +    bool styleWithCSS() const { return m_styleWithCSS; }
> 
> Maybe add "should" to this name?

That sounds better, will fix.


> > +    if (position.node())
> > +        if (Document* document = position.node()->document())
> > +            m_usesLegacyFormattingTags = !document->frame()->editor()->styleWithCSS();
> 
> Should have braces around this.

OK.


> > -StyleChange::ELegacyHTMLStyles StyleChange::styleModeForParseMode(bool isQuirksMode)
> > -{
> > -    return isQuirksMode ? UseLegacyHTMLStyles : DoNotUseLegacyHTMLStyles;
> > -}
> 
> Do we want to preserve the non-quirks mode rule here? If so, then the style
> with CSS flag should default to true for non-quirks documents. We should at
> least have a test case for this.

Most of our big clients (Mail, GMail, Yahoo) appear to use documents in quirks mode by default, so I'm not sure that preserving non-quirks mode behavior would be useful to that many people, especially since it sounds like most people don't want CSS styling.

Justin
Comment 23 Justin Garcia 2009-02-03 15:31:29 PST
(In reply to comment #21)
> or default it to false and match IE behavior (although IE doesn't have this command, 
> so it's effectively always false there).  Either default seems fine to me.

I think I want to match IE and default it to false, to make it easier to preserve behavior in Mail, which currently gets HTML formatting tags b/c they use a document in quirks mode.

Justin
Comment 24 Justin Garcia 2009-02-03 17:19:21 PST
http://trac.webkit.org/changeset/40560