Skip to content

Removing/outdent a line from ordered list causes the following list to start with an incorrect number#62938

Merged
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
jesxilin:eng/312522
Apr 21, 2026
Merged

Removing/outdent a line from ordered list causes the following list to start with an incorrect number#62938
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
jesxilin:eng/312522

Conversation

@jesxilin
Copy link
Copy Markdown
Contributor

@jesxilin jesxilin commented Apr 16, 2026

4216f6e

Removing/outdent a line from ordered list causes the following list to start with an incorrect number
https://bugs.webkit.org/show_bug.cgi?id=312522
rdar://173537449

Reviewed by Aditya Keerthi.

In unlistifyParagraph, there isn't a procedure to set the
starting list number after splitting. So, when it calls
splitElement() to split the list, this calls
Element::cloneElementWithoutChildren which clones all attributes,
including the start attribute.

For example, if the original list started with 11 and the list
is split in the middle, both lists now start with 11. The
expectation is when splitting the list, the second list starts
from 1.

Thus, make a function that splits the list and sets the second starting
list number to be 1 if a start attribute was originally provided. Otherwise,
don't do anything. Now, outdenting, removing list formatting, and entering to
create new lines should all have the following list start with 1.

Additionally, add layout tests for the cases to confirm the correct
starting list numbers. For outdenting and remove list formatting, both
go down the same code path so there is only one test.

* LayoutTests/editing/execCommand/break-out-of-list-resets-start-number-expected.txt: Added.
* LayoutTests/editing/execCommand/break-out-of-list-resets-start-number.html: Added.
* LayoutTests/editing/execCommand/outdent-list-resets-start-number-expected.txt: Added.
* LayoutTests/editing/execCommand/outdent-list-resets-start-number.html: Added.
* Source/WebCore/editing/CompositeEditCommand.cpp:
(WebCore::CompositeEditCommand::splitListElement):
(WebCore::CompositeEditCommand::moveParagraphs):
* Source/WebCore/editing/CompositeEditCommand.h:
* Source/WebCore/editing/InsertListCommand.cpp:
(WebCore::InsertListCommand::unlistifyParagraph):

Canonical link: https://commits.webkit.org/311696@main

a385044

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win ❌ 🛠 ios-apple
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 win-tests ❌ 🛠 mac-apple
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe ✅ 🛠 vision-apple
✅ 🧪 ios-wk2-wpt ✅ 🧪 api-mac-debug ✅ 🛠 gtk3-libwebrtc
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 ios-safer-cpp ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ❌ 🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@jesxilin jesxilin self-assigned this Apr 16, 2026
@jesxilin jesxilin requested a review from rniwa as a code owner April 16, 2026 23:52
@jesxilin jesxilin added the WebKit Misc. For miscellaneous bugs in the WebKit framework (and not JavaScriptCore or WebCore). label Apr 16, 2026
Comment thread Source/WebCore/editing/InsertListCommand.cpp Outdated
Comment thread Source/WebCore/editing/InsertListCommand.cpp Outdated
pxlcoder
pxlcoder previously approved these changes Apr 17, 2026
Copy link
Copy Markdown
Member

@pxlcoder pxlcoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r+ with the one comment addressed.

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Apr 17, 2026
@pxlcoder pxlcoder dismissed their stale review April 17, 2026 06:40

Looks like there's some more work needed here. We should not be adding the start attribute if the list already starts at 1.

@jesxilin jesxilin removed the merging-blocked Applied to prevent a change from being merged label Apr 17, 2026
Comment thread Source/WebCore/editing/InsertListCommand.cpp Outdated
@pxlcoder
Copy link
Copy Markdown
Member

The latest version of the PR makes an additional change to CompositeEditCommand to cover another case, but did not add more test coverage. Can we add another test?

Copy link
Copy Markdown
Member

@pxlcoder pxlcoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J C Made It

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Apr 21, 2026
@jesxilin jesxilin added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed merging-blocked Applied to prevent a change from being merged labels Apr 21, 2026
…o start with an incorrect number

https://bugs.webkit.org/show_bug.cgi?id=312522
rdar://173537449

Reviewed by Aditya Keerthi.

In unlistifyParagraph, there isn't a procedure to set the
starting list number after splitting. So, when it calls
splitElement() to split the list, this calls
Element::cloneElementWithoutChildren which clones all attributes,
including the start attribute.

For example, if the original list started with 11 and the list
is split in the middle, both lists now start with 11. The
expectation is when splitting the list, the second list starts
from 1.

Thus, make a function that splits the list and sets the second starting
list number to be 1 if a start attribute was originally provided. Otherwise,
don't do anything. Now, outdenting, removing list formatting, and entering to
create new lines should all have the following list start with 1.

Additionally, add layout tests for the cases to confirm the correct
starting list numbers. For outdenting and remove list formatting, both
go down the same code path so there is only one test.

* LayoutTests/editing/execCommand/break-out-of-list-resets-start-number-expected.txt: Added.
* LayoutTests/editing/execCommand/break-out-of-list-resets-start-number.html: Added.
* LayoutTests/editing/execCommand/outdent-list-resets-start-number-expected.txt: Added.
* LayoutTests/editing/execCommand/outdent-list-resets-start-number.html: Added.
* Source/WebCore/editing/CompositeEditCommand.cpp:
(WebCore::CompositeEditCommand::splitListElement):
(WebCore::CompositeEditCommand::moveParagraphs):
* Source/WebCore/editing/CompositeEditCommand.h:
* Source/WebCore/editing/InsertListCommand.cpp:
(WebCore::InsertListCommand::unlistifyParagraph):

Canonical link: https://commits.webkit.org/311696@main
@webkit-commit-queue
Copy link
Copy Markdown
Collaborator

Committed 311696@main (4216f6e): https://commits.webkit.org/311696@main

Reviewed commits have been landed. Closing PR #62938 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 4216f6e into WebKit:main Apr 21, 2026
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Apr 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WebKit Misc. For miscellaneous bugs in the WebKit framework (and not JavaScriptCore or WebCore).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants