- From: Aryeh Gregor <ayg@aryeh.name>
- Date: Fri, 5 Aug 2011 13:59:17 -0400
On Thu, Aug 4, 2011 at 5:22 PM, Ryosuke Niwa <rniwa at webkit.org> wrote: > 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 :( Yeah, it's a problem. The basic issue is that there are no standard interfaces to query things like "what's the height of this <br>?" or "are these two nodes in the same line box?" from CSS, so I have nothing to reference and browser engines might not even know the information. So I hack around it with a mix of pure-DOM definitions, and just not defining things clearly. > 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). Hmm, I'm not sure. Text that's visibility: hidden looks the same to the user as text that's color: transparent, so why should it behave differently for selections? > I still don't understand exactly when sibling criteria returns true and > which node new parent instructors returns. ?Where are these algorithm > defined? The "wrap" algorithm takes three inputs: a list of nodes, and two algorithms. The callers provide the sibling criteria and new parent instructions algorithms. Take a look at the callers of the wrap algorithm and hopefully it will be clearer. Do you have any suggestions on how to make this clearer in the spec? Maybe the note I just added at the beginning <http://aryeh.name/spec/editing/editing.html#wrapping-a-list-of-nodes> helps? > 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. That would be nice, but it's hard to figure out sometimes. Like if the selection is in <p>{}<br></p>, and a script does insertHTML to add "foo", we want to remove the <br>, but if it adds "<!-- foo -->" we want to keep it. And if you select <p>{<br>}</p> and insertHTML to add "<!-- foo -->", we need to make a new <br> regardless. I could write it so that it checks after the fact if the <br> is extraneous, but it seemed easier to just remove it in the beginning and then add a new one later if needed. CSS makes me sad sometimes. :( > 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 are complications. For instance, HTML defines <a>, <ins>, and <del> as being phrasing content only if they contain only phrasing content, which isn't useful for our purposes. The idea of phrasing content is meant to be concerned with authoring conformance, and it doesn't seem well suited to what we need. But I'd like to see something worked out in the long term to harmonize them, yeah. It's just not an immediate need, it can wait. > I personally think Gecko and Opera's behavior makes more sense here. I don't. Partly it's for the reason I give in the comment next to backColor's action: it's """ incoherent from a user perspective. For instance, if you try it on paragraphs the background will have big gaps where the margins are. If you try it on an inline element that's a child of the editing host, it will do nothing or apply the background to everything or such, even though such an inline element is visually indistinguishable from one sitting inside a div. This would only make sense if we take considerable effort to ensure that block elements all have no margins, or if we wrap things in a div if they have margins, or something like that. """ background-color on block elements doesn't make sense from a WYSIWYG perspective, because it behaves differently based on whether an element has margin vs. padding, or whether there's a wrapper around a block or just <br>'s, etc. These differences normally aren't visible in rendering, so making command behavior depend on them is confusing. Also, the way the spec defines it is the way both IE and WebKit behave, so that's a large majority of the market and is probably the most compatible. > This is a WebKit bug that I've been intending to > fix:?https://bugs.webkit.org/show_bug.cgi?id=11089. Okay, changed: http://aryeh.name/gitweb.cgi?p=editing;a=commitdiff;h=b599880c > 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 don't like the use of the word "style" here, because it doesn't fit for createLink. I could change it from "state override" and "value override" to "typing state" and "typing value", I guess. But it doesn't just affect typing, either: it also affects the return value of queryCommandState()/queryCommandValue(). Thus "override", because it overrides the normal definition of state/value. >> 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 Yes, WebKit uses 600 as the cutoff for queryCommandState(), and so does the spec. However, on my systems, <span style=font-weight:600> visually displays as bold (i.e., looks the same as <b>) in all browsers except Chrome, where it looks the same as normal text. >> Implementations will have to be rewritten to match the spec, yeah. > > I don't think we can do that in any foreseeable future. Not now, no. For now I expect only gradual changes. Once the spec becomes stable and well-tested, however, I expect it will make sense to do rewrites. The situation is very similar to the HTML5 parser: previously browsers behaved in all sorts of crazy ways, and the only way to really align them was for everyone to do a rewrite. But the spec needs to be a lot stabler and better-tested for that to happen. > 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. We definitely need such a test suite anyway. > Very nice! ?Can we have scores for each component so that? I don't like having scores in test suites. Tests should be a tool to figure out what bugs you have, not a way to claim browser X conforms better or worse than browser Y. In a good test suite, a trivial bug can cause thousands of failures that are irrelevant in real life. For instance, IE and Opera both fail a huge number of my reflection tests because they don't handle nulls correctly in DOM methods, and I test nulls in every string attribute, but those failures aren't important for real websites. So just improve things until the tests all pass, and you're good to go. :) (But my tests aren't very good right now for conformance tests, I have to improve them to make them work better for that.) On Thu, Aug 4, 2011 at 6:47 PM, Ehsan Akhgari <ehsan at mozilla.com> wrote: > Here's my feedback on sections 1-3. ?I'll provide feedback on the rest of > the specification in the near future. Thanks! > * The "gecko" prefix should be changed to "moz", to be compatible with other > Gecko specific prefixes. Yeah, dunno what I was thinking when I wrote that. Fixed: http://aryeh.name/gitweb.cgi?p=editing;a=commitdiff;h=aa0fcd13 > * Do we have any other case where the HTML spec says that a browser can > change its behavior if it's upgraded in the middle of the browsing session? No, probably not. I'm just being paranoid. Specifically, I was thinking of a browser saving its state when it shuts down and restoring when it restarts. It's kind of academic, I could probably just remove the whole paragraph. > * Gecko currently has some problems in returning the correct status of a > command based on the active range, so the results obtained from Firefox here > should not be a determining factor (see > https://bugzilla.mozilla.org/show_bug.cgi?id=676401). Yeah, I noticed that behavior and saw it was crazy, so I wasn't planning on following it. :) I didn't guess the reason, but the reason given in the bug makes sense. Noted: http://aryeh.name/gitweb.cgi?p=editing;a=commitdiff;h=8466d321 > * On the same note, I actually think that WebKit's behavior makes sense. ?We > need to make sure that _something_ happens when the command is executed > after making sure that queryCommandEnabled returns true. ?If the beginning > of the active range falls into an editable section, that would happen > (perhaps to part of the range). ?Initially I thought that it would be better > to return true if any range in the selection falls into the editable > section, but that wouldn't make a lot of sense if the range spans across > multiple contentEditable sections. The thing is, some commands can have effects even if they don't contain any editable nodes. For instance, if you have <div contenteditable>foo<span contenteditable=false>b[a]r</span>baz</div> and you run indent on it, you get <div contenteditable><blockquote>foo<span contenteditable=false>b[a]r</span>baz</blockquote></div>. Should indent (and other block commands) just do nothing in this case? Why? What's the use-case for queryCommandEnabled() anyway, really?
Received on Friday, 5 August 2011 10:59:17 UTC