John A. Bilicki III Reporter
	Description • 13 years ago

User-Agent:       Mozilla/5.0 (Windows NT 5.2; WOW64; rv:2.0.1) Gecko/20100101 Firefox/4.0.1
Build Identifier: 

If you have some XHTML code like this...
<div contenteditable="true" id="editor_test">
<p tabindex="3">here is some text</p>
<p tabindex="3">here is some text</p>
<p tabindex="3">here is some text</p>
</div>

When you blur/tab from the last tabable element before the editor_test id focus is given to the divisible element instead of it's first paragraph[0]. After a couple days of researching I can't seem to determine any standardized method of moving the keyboard caret (not cursor, that is mouse related) to the paragraph element.

In example on my site I make sure all text is within span or p elements otherwise it'll show up with the color #f0f to let me know the XHTML is not correctly formatted.

Reproducible: Always

Steps to Reproduce:
1. Using the tab key blur the last tabable element to give focus to the editable divisible element.

2. Attempt to type a new paragraph without using the arrow keys to navigate to a paragraph element.

Actual Results:  
It is possible to type text in the editable divisible element with no way to use JavaScript to prevent incorrect XHTML formatting.

I have not been able to determine any way to work around this.

Expected Results:  
If paragraph elements exist inside of the divisible element (even a blank one with firstChild.nodeValue.length==0) they should be given focus.

The current behavior is inconsistent with Presto and WebKit, I haven't bothered testing with IE as even IE9 still uses the same proprietary methods and properties as IE6 did in the context of text selection/DOM range to the best of my knowledge.

It would be desirable to allow both paragraph and non-paragraph editing options. In example if a paragraph exists (regardless of whether it is empty or not) I think it would make sense for Firefox to utilize paragraph elements for text. If the webmaster has intentionally left the (in my case divisible element) empty then I think break lines in place of paragraphs would be acceptable then.

I'm not sure what ECMAScript has to say about this (was not able to find selectionStart method in the 5th edition) so I'm guessing there is a bit of ambiguity on the subject.

I tested this in the June 7th 2011 builds of Firefox 4, 5, 6, and 7.

	Boris Zbarsky [:bzbarsky]
	Comment 1 • 13 years ago

For what it's worth, I think the current behavior is correct: both the <div> and the <p> elements should be in the tab order.

	Aryeh Gregor (:ayg) (no longer with Mozilla)
	Comment 2 • 13 years ago

(In reply to comment #0)
> Actual Results:  
> It is possible to type text in the editable divisible element with no way to
> use JavaScript to prevent incorrect XHTML formatting.
> 
> I have not been able to determine any way to work around this.

You could attach an onkeyup handler to the div that wraps any unwrapped text in a <p>.  Not exactly simple to do correctly, but theoretically possible.  Note that lots of other stuff in Gecko can break out of paragraphs.  Like try creating a list, then outdenting it, and you'll no longer be wrapped in a paragraph.

> If paragraph elements exist inside of the divisible element (even a blank
> one with firstChild.nodeValue.length==0) they should be given focus.

You can't really give focus to a paragraph with no children, since it has zero height.  There's nowhere to put the cursor.  (Nothing should be creating such paragraphs, though.)

> The current behavior is inconsistent with Presto and WebKit, I haven't
> bothered testing with IE as even IE9 still uses the same proprietary methods
> and properties as IE6 did in the context of text selection/DOM range to the
> best of my knowledge.

IE9 supports DOM Range, more or less interoperably with other browsers.

(In reply to comment #1)
> For what it's worth, I think the current behavior is correct: both the <div>
> and the <p> elements should be in the tab order.

The <div> should be in the tab order, but if you tab to the <div>, should it really focus right at the beginning?  Or should it focus just inside the first paragraph?  The latter makes more sense.  Gecko is putting the cursor in a place where it's not possible to put it by clicking or arrow-key navigation.  User actions shouldn't be able to create selections like that, or if they can, typing should normalize them to a more reasonable location.  The cursor looks like it's at the beginning of a text node, but actually if you start typing it will suddenly create a new line before the current one and add your text there.

This test shows everyone but Gecko puts the cursor at the beginning of the first text node in a simple case like this:

http://software.hixie.ch/utilities/js/live-dom-viewer/?%3C!DOCTYPE%20html%3E%0A%3Cdiv%20contenteditable%3E%0A%3Cp%3Eabc%3C%2Fp%3E%0A%3C%2Fdiv%3E%0A%3Cscript%3E%0Adocument.querySelector(%22div%22).focus()%3B%0Aw(getSelection().focusNode)%3B%0A%3C%2Fscript%3E

(In IE it logs "null", but it has the same basic behavior.)

All this stuff is unspecified, unfortunately, as far as I can tell.  At some point I plan to define some category of boundary points as "reasonable" and require in DOM Range that UAs not allow user actions to create selections with "unreasonable" boundary points like this.  UAs already mostly don't allow user actions to create weird selections, but they differ in the details and it could use standardization.  (In some common cases it's quite significant -- e.g., if you have <b>foo</b>bar and the user moves the selection to between "foo" and "bar", it should always be inside the <b>, not outside, so that text is bold when the user starts typing.  WebKit gets that case right, IE and Gecko are inconsistent.)

	John A. Bilicki III Reporter
	Comment 3 • 13 years ago

Boris, I have not been able to determine any way to give keyboard caret "focus" to the first paragraph however Presto and WebKit execute this gracefully.

Aryeh, actually I had a LOT of difficulty trying to get IE9 to give caret focus to the editable element however when I was successful it kept the text (and new breaks) inside of paragraph elements as desired so IE9, Chrome and Opera have consistent behavior whereas Firefox is not consistent.

What is important is to remember the context of use for divisible and paragraph elements. A divisible element is not intended to hold text directly, it is a generic block level element intended for visual formatting whereas a paragraph's context for existing is explicitly for containing text and other text related elements.

I do all my testing and work in XHTML served as application/xhtml+xml and so far none of the browsers have generated any malformed XML, not that there won't be instances that I haven't encountered. The main problem is that unless there is some very little known method for moving the keyboard caret inside of an element with the contenteditable attribute it then creates a lot of difficulty to prevent the user from creating non-semantic markup.

Also where does the contenteditable attribute initially appear in? What is the initial specification or was it vendor specific? I was able to find that it started showing up in the very late 90s. Is there any documentation about it's behavior in the context of tabindex attributes? There is a Gecko bug (I think Gecko 1.8 and older offhand) where when a page is served as application/xhtml+xml that you can not hold the tab key down and cycle through all the elements more than once unless every tabable element has a tabindex attribute and I filed that bug a few years ago. My ultimate goal here is to make sure that end users especially those who aren't very technical will be able to choose whatever browser they prefer and be able to have a consistent experience.

	Boris Zbarsky [:bzbarsky]
	Comment 4 • 13 years ago

> What is the initial specification or was it vendor specific? 

The latter; it was originally introduced in IE.

	(no longer active)
	Comment 5 • 13 years ago

(In reply to comment #1)
> For what it's worth, I think the current behavior is correct: both the <div>
> and the <p> elements should be in the tab order.

Boris is right, the existing behavior is correct.

(In reply to comment #3)
> Boris, I have not been able to determine any way to give keyboard caret
> "focus" to the first paragraph however Presto and WebKit execute this
> gracefully.

var p = document.getElementById("foo");
p.focus(); // to give the paragraph focus
window.getSelection().collapse(p,0); // to select the paragraph (which moves the caret to it as a side effect)

I think you're confusing focus with selection.  These are two different concepts, which sometimes align, and other times don't.

(In reply to comment #2)
> > If paragraph elements exist inside of the divisible element (even a blank
> > one with firstChild.nodeValue.length==0) they should be given focus.
> 
> You can't really give focus to a paragraph with no children, since it has
> zero height.  There's nowhere to put the cursor.  (Nothing should be
> creating such paragraphs, though.)

Actually, you can.  Try loading data:text/html,<p contenteditable>, and then pressing tab to focus the paragraph.

	Aryeh Gregor (:ayg) (no longer with Mozilla)
	Comment 6 • 13 years ago

(In reply to comment #5)
> Boris is right, the existing behavior is correct.

It's correct that the <div> should be in the tab order.  It's not correct that tabbing to the div should place the selection inside the div instead of inside its paragraph child.  Example:

data:text/html,<!doctype html><input><div contenteditable><p>Text</div>

Hit tab twice, then hit "A".  In Firefox 5.0a2, this produces <div contenteditable>a<p>Text</div>, with "a" on its own line.  In IE9, Chrome 13 dev, and Opera 11.11, it produces <div contenteditable><p>aText</div>.  The latter behavior makes more sense.  Gecko's behavior means that to the user, it looks like the cursor is before "Text":

  |Text

But then when you type, it jumps to a new line for no apparent reason:

  a|
  
  Text

User actions should not be able to place the cursor in such weird places.  Focusing the contenteditable region should probably put the cursor at the beginning of the first visible text node, or similar.  That seems to be what other browsers do in this case (don't know how they handle tricky cases).

Note that if the contenteditable div is the first focusable item on the page, like data:text/html,<!doctype html><div contenteditable><p>Text</div>, the caret seemingly winds up at the *end* of the last text node in Firefox.  At any rate, the behavior is inconsistent.

> Actually, you can.  Try loading data:text/html,<p contenteditable>, and then
> pressing tab to focus the paragraph.

Works in Firefox, but it's not mouse-focusable, and the caret isn't necessarily visible until you type something.  So maybe you can, but you probably don't want to.

	Boris Zbarsky [:bzbarsky]
	Comment 7 • 13 years ago

Aryeh, with your proposed behavior it becomes impossible for the user to enter text before the first <p>.  Why is that desirable?

> User actions should not be able to place the cursor in such weird places

Why not, if those places are meaningful?  I'd rather argue that not being able to get to those places via arrow keys and clicking is a bug.

	John A. Bilicki III Reporter
	Comment 8 • 13 years ago

Like I said earlier I think the choice should be subjective to the (X)HTML that is already placed there. An empty paragraph element inside the editable divisible element should have the first key pressed display inside of the paragraph element HOWEVER if the web designer does NOT want paragraph elements then I think having no childNodes (e.g. p) inside the editable divisible element should produce the text without paragraphs.

I think a possible ambiguity that may need to be considered is if there is a textNode BEFORE the first paragraph. I think the browser SHOULD incorporate SOME method of intuitive design. In example I think (offhand) that a simple breakline counts as a textNode? I would not personally consider that a legitimate reason to have the text typed appear outside of the paragraph however if there was a single white space character after the opening tag of the divisible element though before the opening tag of the first paragraph element THEN I think it would (by intuitive design) make sense to have the text appear directly inside of the divisible element instead of the paragraph.

So for a visual this is what I propose....

Tab caret to paragraph...

1.)

<div contenteditable="true"><p></p></div>

2.) 
<div contenteditable="true">
<p></p>
</div>

Tab caret BEFORE paragraph without generating new paragraph...

3.) (second line has a space at the beginning)

<div contenteditable="true">
 <p></p></div>

4.)

<div contenteditable="true"> <p></p></div>

I think this would be most intuitive allowing either paragraph or non-paragraph approaches to work. :-)

	Aryeh Gregor (:ayg) (no longer with Mozilla)
	Comment 9 • 13 years ago

(In reply to comment #7)
> Aryeh, with your proposed behavior it becomes impossible for the user to
> enter text before the first <p>.  Why is that desirable?

It's perfectly possible.  Put the cursor right at the beginning of the first paragraph, hit enter, hit up arrow.  Then when you type, the text will go on the line before the first <p>.

Of course, this leaves no way for the user to enter text before the <p> that's not itself wrapped in a <p>.  But there's also no way for the user to set an element's id using plain contenteditable.  It's not meant to allow arbitrary modifications of the DOM, only modifications that correspond to what WYSIWYG editors conventionally permit.  If users want to make arbitrary modifications of the DOM, they should use a tool designed for it, like Firebug.

Not allowing users to add text outside a <p> (or <li> or whatever) is a feature, not a bug.  It allows authors to easily style or otherwise process paragraphs.  Per-paragraph wrappers are useful to have sometimes, and harmless at worst.

And also, Gecko disagrees with all other browsers here.

> Why not, if those places are meaningful?  I'd rather argue that not being
> able to get to those places via arrow keys and clicking is a bug.

What does "meaningful" mean?  From the user perspective, having the cursor in this location is not meaningful at all.  It makes no sense.  It looks like exactly the same location as the start of the text node, but typing has a different effect (making a new line before the current one).  Having different behavior for the two different positions makes some sense for scripting APIs, although in practice the flexibility does add considerable complexity to using Ranges.  But it makes no sense for a visual API, where you can't tell the difference between the positions.

Even if having different behaviors for the two different positions would make sense, the behavior for the before-<p> position makes no sense in itself.  Visually, you're creating an extra line when the user didn't hit Enter or otherwise indicate they want an extra line.  Lines should only be broken when the user hits Enter or similar -- that's what the key is for, after all.

FWIW, I do plan to spec behavior here that would require behavior more or less like the other browsers, although it will take me some time to work out the details when I get to it.  I plan to make a whatwg thread on the subject of weird boundary points in Selections in the not-too-distant future.  (Note that WebKit and Opera don't allow the Selection object to have an endpoint at such a location at all.  Even if you try setting it using addRange(), they'll alter it: <http://software.hixie.ch/utilities/js/live-dom-viewer/saved/1025>.  The spec currently matches IE/Gecko here.)

	(no longer active)
	Comment 10 • 13 years ago

(In reply to comment #9)
> (In reply to comment #7)
> > Aryeh, with your proposed behavior it becomes impossible for the user to
> > enter text before the first <p>.  Why is that desirable?
> 
> It's perfectly possible.  Put the cursor right at the beginning of the first
> paragraph, hit enter, hit up arrow.  Then when you type, the text will go on
> the line before the first <p>.
> 
> Of course, this leaves no way for the user to enter text before the <p> that's
> not itself wrapped in a <p>.  But there's also no way for the user to set an
> element's id using plain contenteditable.  It's not meant to allow arbitrary
> modifications of the DOM, only modifications that correspond to what WYSIWYG
> editors conventionally permit.  If users want to make arbitrary modifications
> of the DOM, they should use a tool designed for it, like Firebug.

I think you have a point, although you're not bringing it up explicitly.  It's not the placement of the selection itself which is problematic.  It's what happens after it (the existing text "jumping down" to make room for the newly entered text) which looks like a bug to me.

> Not allowing users to add text outside a <p> (or <li> or whatever) is a
> feature, not a bug.  It allows authors to easily style or otherwise process
> paragraphs.  Per-paragraph wrappers are useful to have sometimes, and harmless
> at worst.
> 
> And also, Gecko disagrees with all other browsers here.

The only reason that this disagreement is a problem is because of the weird thing that the user would see.  :-)

But do you know how WebKit actually behaves here?  (The exact algorithm that they use?)

That could help us think more concretely about alternative behaviors.

> FWIW, I do plan to spec behavior here that would require behavior more or less
> like the other browsers, although it will take me some time to work out the
> details when I get to it.

Again, I'd like to understand what that behavior is in the general case...

>  I plan to make a whatwg thread on the subject of
> weird boundary points in Selections in the not-too-distant future.  (Note that
> WebKit and Opera don't allow the Selection object to have an endpoint at such a
> location at all.  Even if you try setting it using addRange(), they'll alter
> it: <http://software.hixie.ch/utilities/js/live-dom-viewer/saved/1025>.  The
> spec currently matches IE/Gecko here.)

That's just taking it too far, IMO.  There is no reason to propagate this to the API level.  I think this is a bug in WebKit and Opera, and they should just fix it.

	(no longer active)
	Comment 11 • 13 years ago

(In reply to comment #8)
> I think a possible ambiguity that may need to be considered is if there is a
> textNode BEFORE the first paragraph. I think the browser SHOULD incorporate
> SOME method of intuitive design. In example I think (offhand) that a simple
> breakline counts as a textNode? I would not personally consider that a
> legitimate reason to have the text typed appear outside of the paragraph
> however if there was a single white space character after the opening tag of
> the divisible element though before the opening tag of the first paragraph
> element THEN I think it would (by intuitive design) make sense to have the text
> appear directly inside of the divisible element instead of the paragraph.

This wouldn't work, since these types of whitespaces are collapsible (unless you're using a style such as whitespace: pre), and they're ignored by the browser in almost every case.

	John A. Bilicki III Reporter
	Comment 12 • 13 years ago

Instead of debating back and forth on what the correct behavior might be how about please leaving a response to my comments? I've not only taken the time to file the bug though as a web designer/developer have proposed a balanced solution that would allow everyone to have their cake and eat it to.

	(no longer active)
	Comment 13 • 13 years ago

(In reply to comment #12)
> Instead of debating back and forth on what the correct behavior might be how
> about please leaving a response to my comments? I've not only taken the time to
> file the bug though as a web designer/developer have proposed a balanced
> solution that would allow everyone to have their cake and eat it to.

I did reply to comment 8 (please see comment 11).  Unfortunately that wouldn't be a good solution to the problem at hand.

And I do appreciate you taking the time to file this bug and discuss it with us.  :-)  Unfortunately as it turns out with many web-related stuff, there is no easy answer...

	John A. Bilicki III Reporter
	Comment 14 • 13 years ago

It's an absolutely PERFECT solution, the behavior is controlled by the web designer as it's the web designer who has to deal with the generated code. What is the point of fixing a bug in a context that will automatically generate yet another bug? The usage of paragraph elements should directly correlate to what the first childNode is except for the presence of a  breakline. A tool without a purpose is not a tool. A tool that is difficult to use is nothing more than a trinket. Standards exist because of needs and they evolve around the needs of the people who utilize them. So ultimately the only question I can ask that I can think of that hasn't, is there a third position (besides the current bug) other than what I have proposed?

WebKit / Chrome 12 beta does exactly what I have proposed.

The attached file patches the issue although it introduces a third possibility, moving the caret to the end of the last paragraph element inside the editor. When tabbing to another element and then tabbing back the caret's original position inside the editor is restored regardless of where the mouse cursor is clicked. The point is to allow clients to continue editing from where they left off. The goal of this bug report though it to ensure that if paragraph elements exist inside of the divisible element with the contenteditable attribute that paragraphs should be generated for the user when pressing enter and if no paragraph elements exist inside of the editor then none should be generated. It's exactly how the other browsers behave, even old versions of IE.

	(no longer active)
	Comment 15 • 13 years ago

(In reply to comment #14)
> Created attachment 538981 [details]
> An XHTML contentEditable example.
> 
> It's an absolutely PERFECT solution,

Here's why it's actually not a good solution.  Web authors mostly think that these two constructs should result in the same output visually:

<div>
<p>x</p>
</div>

<div>
 <p>x</p>
</div>

In both cases, the whitespace should be collapsed, and adding the space character in the second example should not change the presentation or behavior of the document.  Think about a web developer who reindents their HTML code only to find out that its behavior is changed with the new indentation!

> the behavior is controlled by the web
> designer as it's the web designer who has to deal with the generated code.
> What is the point of fixing a bug in a context that will automatically
> generate yet another bug? The usage of paragraph elements should directly
> correlate to what the first childNode is except for the presence of a 
> breakline. A tool without a purpose is not a tool. A tool that is difficult
> to use is nothing more than a trinket. Standards exist because of needs and
> they evolve around the needs of the people who utilize them. So ultimately
> the only question I can ask that I can think of that hasn't, is there a
> third position (besides the current bug) other than what I have proposed?

Well, that's what we're trying to figure out.

> WebKit / Chrome 12 beta does exactly what I have proposed.

WebKit's behavior may indeed be more desirable here than Gecko's.  I asked Aryeh for more information on WebKit's behavior because he seems to be more knowledgeable about WebKit than I am.

> The attached file patches the issue although it introduces a third
> possibility, moving the caret to the end of the last paragraph element
> inside the editor. When tabbing to another element and then tabbing back the
> caret's original position inside the editor is restored regardless of where
> the mouse cursor is clicked. The point is to allow clients to continue
> editing from where they left off. The goal of this bug report though it to
> ensure that if paragraph elements exist inside of the divisible element with
> the contenteditable attribute that paragraphs should be generated for the
> user when pressing enter and if no paragraph elements exist inside of the
> editor then none should be generated. It's exactly how the other browsers
> behave, even old versions of IE.

While no behavior will keep everyone happy, putting the selection at the end of an element seems to be controversial (as in, all web browsers seem to prefer to put the selection at the beginning by default).  In fact, I've recently changed Gecko's handling of the textarea element to put the selection to the beginning of its content when the textarea is focused for the first time.  So, even though there is no standard to specify what should exactly happen here, I'd opt for putting the selection at the beginning, given the option.

	Aryeh Gregor (:ayg) (no longer with Mozilla)
	Comment 16 • 13 years ago

(In reply to comment #10)
> I think you have a point, although you're not bringing it up explicitly. 
> It's not the placement of the selection itself which is problematic.  It's
> what happens after it (the existing text "jumping down" to make room for the
> newly entered text) which looks like a bug to me.

Right.  If the selection were the same but it automatically adjusted when the user typed anything, that would be fine too.  Having the selection's boundary points be the same in all browsers would be good too, just for consistency, but it doesn't really matter where exactly they are as long as they behave the same.

> But do you know how WebKit actually behaves here?  (The exact algorithm that
> they use?)
> 
> That could help us think more concretely about alternative behaviors.

No.  At some point relatively soon (probably within a month) I do plan to do some reverse-engineering and write some type of spec.

> That's just taking it too far, IMO.  There is no reason to propagate this to
> the API level.  I think this is a bug in WebKit and Opera, and they should
> just fix it.

I'm not so sure.  I'll post a thread, as I said.

(In reply to comment #15)
> WebKit's behavior may indeed be more desirable here than Gecko's.  I asked
> Aryeh for more information on WebKit's behavior because he seems to be more
> knowledgeable about WebKit than I am.

Only because I've been testing execCommand() for a while in all browsers, FWIW.  I haven't really looked at anyone's source code.




(In reply to comment #12)
> Instead of debating back and forth on what the correct behavior might be how
> about please leaving a response to my comments? I've not only taken the time
> to file the bug though as a web designer/developer have proposed a balanced
> solution that would allow everyone to have their cake and eat it to.

Briefly:

(In reply to comment #3)
> What is important is to remember the context of use for divisible and
> paragraph elements. A divisible element is not intended to hold text
> directly, it is a generic block level element intended for visual formatting
> whereas a paragraph's context for existing is explicitly for containing text
> and other text related elements.

divs are allowed to contain text directly:

http://www.whatwg.org/specs/web-apps/current-work/multipage/grouping-content.html#the-div-element

Follow the link where it says "Content model: Flow content" and observe that flow content includes text.

> I do all my testing and work in XHTML served as application/xhtml+xml and so
> far none of the browsers have generated any malformed XML, not that there
> won't be instances that I haven't encountered.

Browsers all operate with DOMs, and it's nearly impossible to create a DOM that doesn't serialize to XML unless you really try.  All browsers will in some cases create DOMs that don't serialize to HTML and/or are invalid, though (even if provided with valid input).

> The main problem is that
> unless there is some very little known method for moving the keyboard caret
> inside of an element with the contenteditable attribute it then creates a
> lot of difficulty to prevent the user from creating non-semantic markup.

You can use the Selection object to adjust the caret to be wherever you want.  You get a Selection object by calling getSelection(), and you can then do things like .collapse(node, offset) to collapse it to a specific location.  Spec is here:

http://html5.org/specs/dom-range.html#apis-for-the-browsing-context-selection:-the-selection-interface

> Also where does the contenteditable attribute initially appear in? What is
> the initial specification or was it vendor specific? I was able to find that
> it started showing up in the very late 90s.

It was originally IE, now vaguely defined in HTML5, and I'm working on greatly improving the definition to improve interop.

> Is there any documentation about
> it's behavior in the context of tabindex attributes?

No.  It's not standardized nearly to that level of detail (yet).

(In reply to comment #8)
> Like I said earlier I think the choice should be subjective to the (X)HTML
> that is already placed there. An empty paragraph element inside the editable
> divisible element should have the first key pressed display inside of the
> paragraph element HOWEVER if the web designer does NOT want paragraph
> elements then I think having no childNodes (e.g. p) inside the editable
> divisible element should produce the text without paragraphs.

Personally I prefer the IE/Opera approach as default, which wraps everything in <p>s.  That's what I've specced for now.  We had a long discussion about it a while ago:

http://lists.whatwg.org/pipermail/whatwg-whatwg.org/2011-May/031577.html

I might add a feature to let authors switch to a wrapperless mode, like how Gecko behaves now.  This would be in the form of an execCommand() command.

> I think a possible ambiguity that may need to be considered is if there is a
> textNode BEFORE the first paragraph. I think the browser SHOULD incorporate
> SOME method of intuitive design. In example I think (offhand) that a simple
> breakline counts as a textNode?

A visible line break is normally a <br> element, which is an Element node, not text.  A line break in the HTML source usually becomes part of a text node (except in a few cases like right after <pre>).

> I would not personally consider that a
> legitimate reason to have the text typed appear outside of the paragraph
> however if there was a single white space character after the opening tag of
> the divisible element though before the opening tag of the first paragraph
> element THEN I think it would (by intuitive design) make sense to have the
> text appear directly inside of the divisible element instead of the
> paragraph.

Such a space would have no effect on the visual rendering.  Whitespace in HTML in positions like that is collapsed and has no meaning.  We're not going to have it behave differently based on a space that's not visible to the user and which the author probably didn't mean to do anything special.  The author might have just indented the <p> element, with no intent that the whitespace be significant.

Moreover, Firefox's current behavior in this case doesn't make any sense and shouldn't be available in any mode.  Tabbing to the contenteditable area and typing should insert the new text before the first visible text on the same line, not on a new line, and certainly not different depending on whether the first line happens to be wrapped in an element or not.

(In reply to comment #14)
> The usage of paragraph elements should directly
> correlate to what the first childNode is except for the presence of a 
> breakline.

Line breaks in HTML source code are not ever treated differently from other whitespace, except in a few special cases (like <pre> and <textarea>).  Making them different in more cases is not the answer.  Giving authors choice is also not the answer.  The answer is making everything behave reasonably no matter what, and not giving authors the option to opt in to unreasonable behavior.  The difficulty is in defining, in general, what's reasonable -- what behavior changes are needed to fix this bug *and* all similar bugs?

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 17 • 6 years ago

Currently, this inconsistency with the other browsers causes Firefox users cannot use IME in draft-js editor.

As far as I've tested, Edge and Blink work as almost same. So, we should take similar behavior for compatibility.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 18 • 6 years ago

Ehsan:

If you still think that we shouldn't change this behavior, let me know as soon as possible.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 19 • 6 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4e8c27916fa2fd0cda2e4115b9422d7489021dd4

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 20 • 6 years ago

This change will change nsIEditor.beginningOfDocument(). If you disagree with this change, let me know.
https://hg.mozilla.org/try/rev/4e8c27916fa2fd0cda2e4115b9422d7489021dd4

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 21 • 6 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=282806b7f96338f6f82af76861b8548de15aad45

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 22 • 6 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=22a43c877aba5271585452fdc71aaf6b1e111f42

	Jorg K (CEST = GMT+2)
	Comment 24 • 6 years ago

(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #20)
> This change will change nsIEditor.beginningOfDocument(). If you disagree
> with this change, let me know.
Hmm, this is a pretty long bug with a lot of detail. All I can say is that TB uses beginningOfDocument() in C++ and JS code to move the caret into the paragraph at the beginning. Could you please give me some TL;DR examples of what is changing?

I've looked at you test cases at
https://hg.mozilla.org/try/rev/4e8c27916fa2fd0cda2e4115b9422d7489021dd4#l7.21
and didn't see anything that should affect composition in TB.

Personally I'm a little surprised about
1) "<p></p><p>[]abc</p>"
2) "<p></p><p>{}<br></p>"
3) "<p> </p><p>[]abc</p>"
4) "<p> </p><p>{}<br></p>"

However, since the empty (or white-space only) paragraphs aren't rendered, positioning into them and then typing would be a little surprising since a new line would appear from nowhere.

	Makoto Kato [:m_kato]
	Comment 25 • 6 years ago
mozreview-review

Comment on attachment 8955510 [details]
Bug 662591 - HTMLEditor should set caret to start of first editable text node or before first editable inline node

https://reviewboard.mozilla.org/r/224668/#review230878

Why don't you create virtual HTMLEditor::InitializeSelection for this?  It code is too complex as C++ code.

	Makoto Kato [:m_kato]
	Comment 26 • 6 years ago
mozreview-review

Comment on attachment 8955510 [details]
Bug 662591 - HTMLEditor should set caret to start of first editable text node or before first editable inline node

https://reviewboard.mozilla.org/r/224668/#review230880

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 27 • 6 years ago

(In reply to Jorg K (GMT+1) from comment #24)
> (In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #20)
> > This change will change nsIEditor.beginningOfDocument(). If you disagree
> > with this change, let me know.
> Hmm, this is a pretty long bug with a lot of detail. All I can say is that
> TB uses beginningOfDocument() in C++ and JS code to move the caret into the
> paragraph at the beginning. Could you please give me some TL;DR examples of
> what is changing?

Almost all of my patch's changes are explained by the commit message.

> This patch makes editor behave as:
> * if selection is already in the editing host except start of the editing host, does nothing.
> * if there is non-editable element before any editable node, move caret to start of the editing host.
> * if there is editable text node or element node which cannot have a text node, move its start or before it.
> * if there is no editable nodes which can contain text nodes, move caret to start of the editing host.

You must be able to ignore the first one because nsIEditor.beginningOfDocument() ignores current selection.

And also the second case too because Thunderbird's editor must not have non-editable area since it must use designMode instead of contenteditable.

The 3rd part is the biggest change from current behavior. This is what we need to fix ASAP for compatibility with the other browsers on Draft.js.

> Personally I'm a little surprised about
> 1) "<p></p><p>[]abc</p>"
> 2) "<p></p><p>{}<br></p>"
> 3) "<p> </p><p>[]abc</p>"
> 4) "<p> </p><p>{}<br></p>"
> 
> However, since the empty (or white-space only) paragraphs aren't rendered,
> positioning into them and then typing would be a little surprising since a
> new line would appear from nowhere.

I also feels both behaviors are odd since even if block is empty, it's rendered as an empty line. So, the tests mean that Gecko collapse selection to start of second line in those cases.

However, current Gecko's behavior in those cases are also odd. Gecko collapses selection to before the first <p> with current build. Therefore, if user types something after moving focus with keyboard or JS sets focus with Element.focus(), user will see text outside of the first paragraph.

If you need not to change some behavior of beginningOfDocument() only in some cases, let me know. I could add optional argument to the new internal method and share most code in it.

Here is the result of the new tests with current Gecko's behavior:

> editor should set selection to start of the text node
> assert_equals: expected "[]abc" but got "{}abc"

> editor should set selection to start of the text node in the <p> node
> assert_equals: expected "<p>[]abc</p>" but got "{}<p>abc</p>"

> editor should set selection to before the <br> node in the <p> node
> assert_equals: expected "<p>{}<br></p>" but got "{}<p><br></p>"

> editor should set selection to before the first <br> node in the <p> node
> assert_equals: expected "<p>{}<br><br></p>" but got "{}<p><br><br></p>"

> editor should set selection to start of the text node in the <span> node
> assert_equals: expected "<span>[]abc</span>" but got "{}<span>abc</span>"

> editor should set selection to before the <br> node in the <span> node
> assert_equals: expected "<span>{}<br></span>" but got "{}<span><br></span>"

> editor should set selection to before the first <br> node in the <span> node
> assert_equals: expected "<span>{}<br><br></span>" but got "{}<span><br><br></span>"

> editor should set selection to start of the text node in the second <span> node
> assert_equals: expected "<span></span><span>[]abc</span>" but got "{}<span></span><span>abc</span>"

> editor should set selection to before the <br> node in the second <span> node
> assert_equals: expected "<span></span><span>{}<br></span>" but got "{}<span></span><span><br></span>"

> editor should set selection to start of the text node in the first <span> node #1
> assert_equals: expected "<span>[]abc</span><span>abc</span>" but got "{}<span>abc</span><span>abc</span>"

> editor should set selection to start of the text node in the first <span> node #2
> assert_equals: expected "<span>[]abc</span><span><br></span>" but got "{}<span>abc</span><span><br></span>"

> editor should set selection to before the <br> node in the first <span> node #1
> assert_equals: expected "<span>{}<br></span><span><br></span>" but got "{}<span><br></span><span><br></span>"

> editor should set selection to before the <br> node in the first <span> node #2
> assert_equals: expected "<span>{}<br></span><span>abc</span>" but got "{}<span><br></span><span>abc</span>"

> editor should set selection to start of the text node in the second <span> node since the text node in the first <span> node is only whitespaces
> assert_equals: expected "<span> </span><span>[]abc</span>" but got "{}<span> </span><span>abc</span>"

> editor should set selection to before the <br> node in the second <span> node since the text node in the first <span> node is only whitespaces
> assert_equals: expected "<span> </span><span>{}<br></span>" but got "{}<span> </span><span><br></span>"

> editor should set selection to start of the text node in the second <span> node even if there is a whitespace only text node before the first <span> node
> assert_equals: expected " <span></span><span>[]abc</span>" but got "{} <span></span><span>abc</span>"

> editor should set selection to before the <br> node in the second <span> node even if there is a whitespace only text node before the first <span> node
> assert_equals: expected " <span></span><span>{}<br></span>" but got "{} <span></span><span><br></span>"

> editor should set selection to start of the text node in the second <p> node
> assert_equals: expected "<p></p><p>[]abc</p>" but got "{}<p></p><p>abc</p>"

> editor should set selection to before the <br> node in the second <p> node
> assert_equals: expected "<p></p><p>{}<br></p>" but got "{}<p></p><p><br></p>"

> editor should set selection to start of the text node in the first <p> node #1
> assert_equals: expected "<p>[]abc</p><p>abc</p>" but got "{}<p>abc</p><p>abc</p>"

> editor should set selection to start of the text node in the first <p> node #2
> assert_equals: expected "<p>[]abc</p><p><br></p>" but got "{}<p>abc</p><p><br></p>"

> editor should set selection to before the <br> node in the first <p> node #1
> assert_equals: expected "<p>{}<br></p><p><br></p>" but got "{}<p><br></p><p><br></p>"

> editor should set selection to before the <br> node in the first <p> node #2
> assert_equals: expected "<p>{}<br></p><p>abc</p>" but got "{}<p><br></p><p>abc</p>"

> editor should set selection to start of the text node in the second <p> node since the text node in the first <p> node is only whitespaces
> assert_equals: expected "<p> </p><p>[]abc</p>" but got "{}<p> </p><p>abc</p>"

> editor should set selection to before the <br> node in the second <p> node since the text node in the first <p> node is only whitespaces
> assert_equals: expected "<p> </p><p>{}<br></p>" but got "{}<p> </p><p><br></p>"

> editor should set selection to start of the text node in the second <p> node even if there is a whitespace only text node before the first <p> node
> assert_equals: expected " <p></p><p>[]abc</p>" but got "{} <p></p><p>abc</p>"

> editor should set selection to before the <br> node in the second <p> node even if there is a whitespace only text node before the first <p> node
> assert_equals: expected " <p></p><p>{}<br></p>" but got "{} <p></p><p><br></p>"

> editor should set selection to start of the text node in the <span> node in the second <p> node
> assert_equals: expected "<p><span></span></p><p><span>[]abc</span></p>" but got "{}<p><span></span></p><p><span>abc</span></p>"

> editor should set selection to before the <br> node in the <span> node in the second <p> node
> assert_equals: expected "<p><span></span></p><p><span>{}<br></span></p>" but got "{}<p><span></span></p><p><span><br></span></p>"

> editor should set selection to start of the text node in the <span> node in the first <p> node #1
> assert_equals: expected "<p><span>[]abc</span></p><p><span>abc</span></p>" but got "{}<p><span>abc</span></p><p><span>abc</span></p>"

> editor should set selection to start of the text node in the <span> node in the first <p> node #2
> assert_equals: expected "<p><span>[]abc</span></p><p><span><br></span></p>" but got "{}<p><span>abc</span></p><p><span><br></span></p>"

> editor should set selection to before the <br> node in the <span> node in the first <p> node #1
> assert_equals: expected "<p><span>{}<br></span></p><p><span><br></span></p>" but got "{}<p><span><br></span></p><p><span><br></span></p>"

> editor should set selection to before the <br> node in the <span> node in the first <p> node #2
> assert_equals: expected "<p><span>{}<br></span></p><p><span>abc</span></p>" but got "{}<p><span><br></span></p><p><span>abc</span></p>"

> editor should set focus to the first editable text node in the first <span> node even if followed by a non-editable node
> assert_equals: expected "<span>[]abc</span><span contenteditable=\"false\"></span>" but got "{}<span>abc</span><span contenteditable=\"false\"></span>"

> editor should set focus to the first editable text node in the first <span> node even if followed by a non-editable node having another text node
> assert_equals: expected "<span>[]abc</span><span contenteditable=\"false\">def</span>" but got "{}<span>abc</span><span contenteditable=\"false\">def</span>"

> editor should set focus to the first editable text node in the first <span> node even if followed by a non-editable node having a <br> node
> assert_equals: expected "<span>[]abc</span><span contenteditable=\"false\"><br></span>" but got "{}<span>abc</span><span contenteditable=\"false\"><br></span>"

> editor should set focus to the first editable <br> node in the first <span> node even if followed by a non-editable node
> assert_equals: expected "<span>{}<br></span><span contenteditable=\"false\"></span>" but got "{}<span><br></span><span contenteditable=\"false\"></span>"

> editor should set focus to the first editable <br> node in the first <span> node even if followed by a non-editable node having a text node
> assert_equals: expected "<span>{}<br></span><span contenteditable=\"false\">abc</span>" but got "{}<span><br></span><span contenteditable=\"false\">abc</span>"

> editor should set focus to the first editable <br> node in the first <span> node even if followed by a non-editable node having a <br> node
> assert_equals: expected "<span>{}<br></span><span contenteditable=\"false\"><br></span>" but got "{}<span><br></span><span contenteditable=\"false\"><br></span>"

> editor should set focus to the first editable text node in the first <p> node even if followed by a non-editable node
> assert_equals: expected "<p>[]abc</p><p contenteditable=\"false\"></p>" but got "{}<p>abc</p><p contenteditable=\"false\"></p>"

> editor should set focus to the first editable text node in the first <p> node even if followed by a non-editable node having another text node
> assert_equals: expected "<p>[]abc</p><p contenteditable=\"false\">def</p>" but got "{}<p>abc</p><p contenteditable=\"false\">def</p>"

> editor should set focus to the first editable text node in the first <p> node even if followed by a non-editable node having a <br> node
> assert_equals: expected "<p>[]abc</p><p contenteditable=\"false\"><br></p>" but got "{}<p>abc</p><p contenteditable=\"false\"><br></p>"

> editor should set focus to the first editable <br> node in the first <p> node even if followed by a non-editable node
> assert_equals: expected "<p>{}<br></p><p contenteditable=\"false\"></p>" but got "{}<p><br></p><p contenteditable=\"false\"></p>"

> editor should set focus to the first editable <br> node in the first <p> node even if followed by a non-editable node having a text node
> assert_equals: expected "<p>{}<br></p><p contenteditable=\"false\">abc</p>" but got "{}<p><br></p><p contenteditable=\"false\">abc</p>"

> editor should set focus to the first editable <br> node in the first <p> node even if followed by a non-editable node having a <br> node
> assert_equals: expected "<p>{}<br></p><p contenteditable=\"false\"><br></p>" but got "{}<p><br></p><p contenteditable=\"false\"><br></p>"

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 28 • 6 years ago
mozreview-review-reply

Comment on attachment 8955510 [details]
Bug 662591 - HTMLEditor should set caret to start of first editable text node or before first editable inline node

https://reviewboard.mozilla.org/r/224668/#review230878

Hmm, I tried to some versions of such patch. However, it's not so good for overriding full of the method.

So, perhaps, making a wrapper method just to set selection ancestor limit as virtual method must be the best. Then, only the additional path for HTMLEditor is moved into HTMLEditor.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 30 • 6 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=463706bf886cacf5d26d4dbabf8fea8c9d99c0b0

	Jorg K (CEST = GMT+2)
	Comment 31 • 6 years ago

(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #27)
> > Personally I'm a little surprised about
> > 1) "<p></p><p>[]abc</p>"
[snip]
> I also feels both behaviors are odd since even if block is empty, it's
> rendered as an empty line. So, the tests mean that Gecko collapse selection
> to start of second line in those cases.
Hmm, I don't follow this. I created a test case with
  <div contenteditable><p></p><p>[]abc</p></div>
and I only see one line, the one with []abc.

> If you need not to change some behavior of beginningOfDocument() only in
> some cases, let me know. I could add optional argument to the new internal
> method and share most code in it.
Since we mostly use <p><br></p> for empty paragraphs, I don't expect any problems. Thanks for the offer, I'll check our tests once this has landed.

	John A. Bilicki III Reporter
	Comment 32 • 6 years ago

First off thank you Masayuki for taking on this bug!

Secondly Gecko has lots of problems with the XML parser than with the HTML parser so if you could please confirm the changes also work when the page is rendered as application/xhtml+xml to avoid having the bug report be split in the future it'd saved a lot of people aggravation. I have some good XML code on the following paragraph...

I did discover how to move the keyboard caret using JavaScript and it was not intuitive to say the least! I posted a repository here: https://github.com/jabcreations/move-keyboard-caret/blob/master/index.xhtml

	Makoto Kato [:m_kato]
	Comment 33 • 6 years ago
mozreview-review

Comment on attachment 8955510 [details]
Bug 662591 - HTMLEditor should set caret to start of first editable text node or before first editable inline node

https://reviewboard.mozilla.org/r/224668/#review231154

::: editor/libeditor/HTMLEditor.cpp:537
(Diff revision 2)
> +  // in HTMLEditor.
> +  bool tryToCollapseSelectionAtFirstEditableNode = true;
> +  if (aSelection.RangeCount() == 1 && aSelection.IsCollapsed()) {
> +    Element* editingHost = GetActiveEditingHost();
> +    nsRange* range = aSelection.GetRangeAt(0);
> +    if (range &&

Is there a situation when range is nullptr wiht RangeCount == 1?

::: editor/libeditor/HTMLEditor.cpp:584
(Diff revision 2)
> -  // Find first editable thingy
> -  bool done = false;
> -  nsCOMPtr<nsINode> curNode = rootElement.get(), selNode;
> -  int32_t curOffset = 0, selOffset = 0;
> -  while (!done) {
> -    WSRunObject wsObj(this, curNode, curOffset);
> +  // If selection range is already in the editing host and the range is not
> +  // start of the editing host, we shouldn't reset selection.  E.g., window
> +  // is activated when the editor had focus before inactivated.
> +  if (aIgnoreIfSelectionInEditingHost && selection->RangeCount() == 1) {
> +    nsRange* range = selection->GetRangeAt(0);
> +    MOZ_DIAGNOSTIC_ASSERT(range);

Is it necessary to use MOZ_DIAGNOSTIC_ASSERT?  RangeCount is 1, so index 0 isn't null.  (If there is a situation on returning null with index=0, please ignore this comment).

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 34 • 6 years ago
mozreview-review-reply

Comment on attachment 8955510 [details]
Bug 662591 - HTMLEditor should set caret to start of first editable text node or before first editable inline node

https://reviewboard.mozilla.org/r/224668/#review231154

> Is there a situation when range is nullptr wiht RangeCount == 1?

According to *current* Selection implementation, it's impossible.

> Is it necessary to use MOZ_DIAGNOSTIC_ASSERT?  RangeCount is 1, so index 0 isn't null.  (If there is a situation on returning null with index=0, please ignore this comment).

As I said above, we won't hit the assertion with *current* Selection implementation.

I guess that should Selection has |nsRange& RangeAt()| which crashes if given index is out of range?

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 35 • 6 years ago

(In reply to Jorg K (GMT+1) from comment #31)
> (In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #27)
> > > Personally I'm a little surprised about
> > > 1) "<p></p><p>[]abc</p>"
> [snip]
> > I also feels both behaviors are odd since even if block is empty, it's
> > rendered as an empty line. So, the tests mean that Gecko collapse selection
> > to start of second line in those cases.
> Hmm, I don't follow this. I created a test case with
>   <div contenteditable><p></p><p>[]abc</p></div>
> and I only see one line, the one with []abc.

Ah, I see. If first <p> is empty and it does not have border nor padding, its margin-top, margin-bottom and the following margin-top are collapsed. So, it causes the first <p> invisible actually.

> > If you need not to change some behavior of beginningOfDocument() only in
> > some cases, let me know. I could add optional argument to the new internal
> > method and share most code in it.
> Since we mostly use <p><br></p> for empty paragraphs, I don't expect any
> problems. Thanks for the offer, I'll check our tests once this has landed.

Thanks. I'll land the patch soon. If you get some regressions, cc me to the regression report. Perhaps, the new method should have additional argument and HTMLEditor::BeginningOfDocument() should customize the behavior with the argument.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 36 • 6 years ago

(In reply to John A. Bilicki III from comment #32)
> First off thank you Masayuki for taking on this bug!

You're welcome and sorry for the long delay for your report.

> Secondly Gecko has lots of problems with the XML parser than with the HTML
> parser so if you could please confirm the changes also work when the page is
> rendered as application/xhtml+xml to avoid having the bug report be split in
> the future it'd saved a lot of people aggravation. I have some good XML code
> on the following paragraph...

The patch does not depend on if the document is an HTML document or an XHTML document since editor modifies DOM tree with DOM APIs.

> I did discover how to move the keyboard caret using JavaScript and it was
> not intuitive to say the least! I posted a repository here:
> https://github.com/jabcreations/move-keyboard-caret/blob/master/index.xhtml

Hmm, I don't understand why don't you use Selection.collapse(Node? node, optional unsigned long offset = 0) and I'm not sure what are bugs of us. Please file new bugs for each symptom.

	Pulsebot
	Comment 38 • 6 years ago

Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/575562183a3b
HTMLEditor should set caret to start of first editable text node or before first editable inline node r=m_kato

	Tiberius Oros[:tiberius_oros]
	Comment 39 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/575562183a3b

	Jorg K (CEST = GMT+2)
	Comment 40 • 6 years ago

(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #35)
> Thanks. I'll land the patch soon. If you get some regressions, cc me to the
> regression report.
No test failures in TB due to this. We don't have tests for everything, so I'll keep an eye out.