Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define behavior of pressing enter in inline editing host #430

Open
foolip opened this issue May 17, 2023 · 12 comments
Open

Define behavior of pressing enter in inline editing host #430

foolip opened this issue May 17, 2023 · 12 comments
Assignees

Comments

@foolip
Copy link
Member

foolip commented May 17, 2023

There are tentative tests for this:
https://github.com/web-platform-tests/wpt/blob/master/editing/other/insertparagraph-in-inline-editing-host.tentative.html
https://wpt.fyi/results/editing/other?label=experimental&label=master&product=chrome&product=firefox&product=safari&aligned&view=interop&q=label%3Ainterop-2023-webcompat

The tests are part of the Interop 2023 Web Compat focus area, but there's no spec for it.

If the behavior can be written down in some specification (whatever its standards track status) managed by the Editing WG, it will be much easier to implement something in Chromium that we can be confident matches the consensus of the WG.

(To reverse engineer the exact behavior from the tests isn't generally possible unless tests cover all possibilities.)

@foolip
Copy link
Member Author

foolip commented May 17, 2023

@chrishtr can you help get this on the Editing WG agenda?

It would be good to have an editing expert read the test and summarize in simpler terms what the important aspects are. I think it has to do with inserting a different element to represent the new line/paragraph depending on what the parent element is, to not end up with <span><div></div><span> for example.

@chrishtr chrishtr added the Agenda+ Agenda item to be inserted in the Editing TF meeting queue label May 17, 2023
@masayuki-nakano
Copy link
Collaborator

I think it has to do with inserting a different element to represent the new line/paragraph depending on what the parent element is, to not end up with <span><div></div><span> for example.

I intended so.

I think insertParagraph should not insert a default paragraph separator <div> or <p>, if there is no proper editable ancestor at insertion point. In the case, the command should work as insertLineBreak (it seems that \n should be inserted if it's preserved by white-space, other wise, <br>).

@foolip
Copy link
Member Author

foolip commented May 19, 2023

Thanks for explaining @masayuki-nakano! How should "proper editable ancestor" be defined here?

@masayuki-nakano
Copy link
Collaborator

I guess that these steps may roughly match with existing insertParagraph command behavior of the browsers:

  1. Let inclusive ancestor block be null
  2. Let container be focus node of selection (after deleting selected content)
  3. If container can not have the new default paragraph as a child
    3-1. If container creates a context, e.g., <caption>, <body>, etc, break this loop
    3-2. If parent node of container is not editable, clear container and break this loop
    3-3. If parent node of container is a <div>, <p>, <hn>, <li>, <dt> or <dd>, break this loop
    3-4. Set container to the parent, then back to the step 3
  4. If container is not set, fallback to insertLineBreak and return
  5. If container is an element creating a context (i.e., broken the loop at 3-1), put the default paragraph in it or insert a line break instead if it cannot have the new paragraph, then, return
  6. Duplicate container and put the clone after it (and delete unnecessary content)

@chrishtr
Copy link

chrishtr commented Jun 8, 2023

3. 3-1. If container creates a context, e.g., <caption>, <body>, etc, break this loop

Hi @masayuki-nakano, could you clarify what you mean by "context" here?

3. If container can not have the new default paragraph as a child

Could you say what conditions lead to "can not have the new default paragraph as a child"? Does this mean "if the container is inline"? If so, would that be determined by tag name or actual inline style?

3. 3-3. If parent node of container is a <div>, <p>, <hn>, <li>, <dt> or <dd>, break this loop

Could you say more about why this specific list of elements?

@masayuki-nakano
Copy link
Collaborator

  1. 3-1. If container creates a context, e.g., <caption>, <body>, etc, break this loop

Hi @masayuki-nakano, could you clarify what you mean by "context" here?

I meant that it creates a things which are enclosed in a group of content, it should not be duplicated like <caption>, etc. (I'm not sure the good term to explain such elements.)

  1. If container can not have the new default paragraph as a child

Could you say what conditions lead to "can not have the new default paragraph as a child"? Does this mean "if the container is inline"? If so, would that be determined by tag name or actual inline style?

I'm thinking about the relation between parent element and child element (new default paragraph) from HTML spec point of view.

  1. 3-3. If parent node of container is a <div>, <p>, <hn>, <li>, <dt> or <dd>, break this loop

Could you say more about why this specific list of elements?

They won't have new default paragraphs in any browsers, I think. They will become the left node, and browsers put the right node for it.

@chrishtr
Copy link

I discussed it within Chromium and we think that the algorithm can be simplified to insert a simple line break (<br>) if one of the following holds:

  • There is no enclosing block of the selection position
  • The enclosing block is one of these:
  • An <hr> element

This is implemented in Chromium here.

The part about phrasing content was added recently, and has not yet been successfully shipped to the stable channel. It's based on an equivalent change to WebKit.

@masayuki-nakano
Copy link
Collaborator

@chrishtr What is the "enclosing block" definition? Which one if <div><span contenteditable>{}<br><span></div>? Sounds like the non-editable <div> in this case.

@chrishtr
Copy link

chrishtr commented Jul 26, 2023

@chrishtr What is the "enclosing block" definition? Which one if <div><span contenteditable>{}<br><span></div>? Sounds like the non-editable <div> in this case.

It's the highest editable ancestor element in the DOM which is a block. If the {} in your example is the caret position, then there would be no enclosing block, because the span element is not a block, and no ancestor of the span is editable.

@johanneswilm johanneswilm removed the Agenda+ Agenda item to be inserted in the Editing TF meeting queue label Aug 10, 2023
@johanneswilm johanneswilm added the Agenda+ Agenda item to be inserted in the Editing TF meeting queue label Sep 1, 2023
@johanneswilm
Copy link
Contributor

TPAC 2023
RESOLUTION: Simon will document what happens in the three browser engines and check existing wpt tests. Then pick one of the behaviors.

@johanneswilm johanneswilm removed the Agenda+ Agenda item to be inserted in the Editing TF meeting queue label Oct 12, 2023
@foolip
Copy link
Member Author

foolip commented Feb 15, 2024

Was there some further update on this? Is Simon @zcorpan or another Simon?

@zcorpan
Copy link
Member

zcorpan commented Feb 15, 2024

It was probably me! Apologies for the delay.

The tests referenced in OP pass now! So it seems we already have interoperable behavior and the spec needs updating to reflect what is implemented.

@zcorpan zcorpan self-assigned this Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants