[whatwg] [editing] HTML Editing APIs specification ready for implementer feedback

On Wed, Aug 3, 2011 at 1:13 PM, Aryeh Gregor <ayg at aryeh.name> wrote:
>
>  On Tue, Aug 2, 2011 at 8:31 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:
> > Feedback on selections 5 through 7:
> >
> > The definition of collapsed line break isn't clear.  Maybe it's br
> > immediately before the end of a block?
>
> Unfortunately, that's not good enough if we want it to be correct in
> all cases.  For instance, the <br> in <p><span><br><!-- foo
> --></span></p> behaves like a collapsed line break for CSS purposes.
> Maybe we could just let the algorithm be wrong in those cases, if we
> can't come up with a better definition.  The problem is that a real
> definition would depend very heavily on CSS.  This is why there's a
> big red box in the spec . . .
>

I see.  It's an interesting point.  In WebKit, we're quite inconsistent in
relying on CSS/rendering engine and pure DOM.  This is an inherent issue in
RTE because user would like the editor to work like WYSIWYG yet we have to
produce conforming markup :(

visibility: hidden shouldn't be taken into account, I don't think.

Such nodes still take up space, they just don't get painted, so they
> don't behave like real invisible nodes.


Right.  But you definitely don't want to place a caret / selection end point
inside a node with visibility : hidden.  So to that extent, you'd have to
mention it somewhere (definitely when you're normalizing selection end
points).

> Step 3 in "remove extraneous line breaks before" seems redundant because
> we
> > traverse the tree in the reversed tree order in step 4.
>
> I'm not sure what you mean.  Step 3 is "While ref has children, set
> ref to its lastChild", and step 4 is "While ref is invisible but not
> an extraneous line break, and ref does not equal node's parent, set
> ref to the node before it in tree order".


Ok.  I misunderstood your algorithm then.

"""
> To wrap a list node list of consecutive sibling nodes, run the
> following algorithm. In addition to node list, the algorithm accepts
> two inputs: an algorithm sibling criteria that accepts a node as input
> and outputs a boolean, and an algorithm new parent instructions that
> accepts nothing as input and outputs a node or null. If not provided,
> sibling criteria returns false and new parent instructions returns
> null.
> """
>
> I also changed "sibling criteria" and "new parent instructions" to use
> <var> instead of <dfn>/<span>, to match other variables.  Does that
> make it clear?
> http://aryeh.name/gitweb.cgi?p=editing;a=commitdiff;h=2992f823


I still don't understand exactly when sibling criteria returns true and
which node new parent instructors returns.  Where are these algorithm
defined?

> Also where would new parent be inserted if new parent's parent was not
> null?
> >  Or will it stay where it was?
>
> I clarified in a comment:
> http://aryeh.name/gitweb.cgi?p=editing;a=commitdiff;h=5771e7c0

Does it make sense now?
>

Yes!  Thank you.

Basically every time I add a <br>, it's because I found a case in
> tests where there was some bug otherwise.  As far as I know, every
> time a <br> is added, it's needed to stop two lines from running
> together -- I avoid adding unnecessary <br>s and in fact remove them
> in a lot of places.  If you look closely at the conditions, these
> <br>'s will only be added when you're wrapping in block elements.
> Things like if you have
>
>  <blockquote>foo</blockquote>[bar]
>
> and the author runs "indent", it has to become
>
>  <blockquote>foo<br>[bar]</blockquote>
>
> not
>
>  <blockquote>foo[bar]</blockquote>
>
> None of these should normally do anything if you're wrapping in
> something like a <b>.  Are there any specific cases that you think are
> unnecessary?
>

Not really. Surprisingly or not, it's a very common technique used
throughout WebKit's editing code.   I just wished we could avoid adding br
if we were to only remove them later.

The biggest problem with phrasing content is that it's only defined for
> valid elements, but we need to also care about things like <font> for
> compatibility.
>

I see.  That's very unfortunate.  Can we defined the list in terms of the
phrasing content though?  Or define phrasing content in terms of your
definition?

There's no interop on what backColor does, and I wound up defining it
> identically to hiliteColor.  This basically matches the behavior of IE and
> WebKit, but Gecko and Opera behave differently.
>

I personally think Gecko and Opera's behavior makes more sense here.

I honor vertical-align because WebKit will currently create such markup in
> styleWithCSS = true mode, although the spec says not to, so I wanted
> the algorithm to handle the existing markup correctly.  If WebKit
> wants to change, I'd be happy to remove this from the spec, since it
> complicates things.
>

This is a WebKit bug that I've been intending to fix:
https://bugs.webkit.org/show_bug.cgi?id=11089.

This seems like it's the same basic idea as "state override" and
> "value override".  I clarified what those are meant to do, since it
> probably didn't make sense on a first reading:
>
> http://aryeh.name/gitweb.cgi?p=editing;a=commitdiff;h=22697d3d
>
> Do you think anything else needs to be changed about how these work?
>

I don't think so although I might have missed something.  I just think that
introducing the concept of "typing style" might make the spec easier to read
and understand.

I think it's simpler not to.  The worst that happens is we replace one
> transparent color by another, which doesn't matter, so there's no
> point in making it more complicated.
>

Okay.

See the comment next to "inline command activated values" for bold.
> On my test systems with default settings (both Ubuntu 10.04 and
> Windows 7 IIRC), 600 and up translates to bold in IE, Firefox, and
> Opera, and 700 and up translates to bold in Chrome.


How did you test that?  As far as I know, WebKit considers 600 to be bold as
well:
http://trac.webkit.org/browser/trunk/Source/WebCore/editing/EditingStyle.cpp?rev=87952#L1024

queryCommandState will definitely return true when font-weight is 600 in the
following example:
http://simple-rte.rniwa.com/?editor=hello%20world&designmode=false&style=font-weight%3A%20600%3B&script=document.getElementById%28%27editor%27%29.focus%28%29%3B

> I'm mainly concerned that there doesn't seem to be a good way for me to
> > check whether the current implementation is consistent with your spec
> > because the spec is defined in terms of algorithms.  Indeed, it's a
> NP-hard
> > problem :(
>
> Implementations will have to be rewritten to match the spec, yeah.
>

I don't think we can do that in any foreseeable future.


> They're not going to give the same results if they weren't written
> from scratch to behave like the spec.  A good test suite should be
> able to check if implementations are matching the spec well enough.
>

Yeah, I think having a good test suite will help us improving our code base
and match the spec to a reasonable degree.

The RTE2 test suite needs to have some tolerance because otherwise it
> would just fail almost every browser for almost every test.  Once we
> have an exact spec, I don't see why browsers shouldn't match it
> exactly.


But I don't think WebKit or any other browser vendors are ready to rewrite
the entire editing engine.  We need a way to gradually improve our code to
match the spec, and a comprehensive test suite will just do that.

I wrote my own tests, which formed the basis for a lot of the spec:


> http://aryeh.name/spec/editing/autoimplementation.html
>

Very nice!  Can we have scores for each component so that?

- Ryosuke

Received on Thursday, 4 August 2011 14:22:42 UTC