[Writing Tools] Make Table: Mail: Generated table has extra borderlines #30564

rr-codes · 2024-07-08T14:45:38Z

`292bb3d`

[Writing Tools] Make Table: Mail: Generated table has extra borderlines
https://bugs.webkit.org/show_bug.cgi?id=276315
rdar://129926389

Reviewed by Aditya Keerthi.

When `didReceiveText` is invoked multiple times when replacing text with a table, multiple table
elements were being created. This is because when the controller tries to re-create the context range
after the first table, it is unable to do so since selections cannot encompass a table or list element.

Fix by not trying to re-create the context range at all; instead, just undo the previous replacements,
and then the context range will always just be the original range for the current composition.

To facilitate this, add a 'silent' option when undo-ing a composition edit command so that a command can
be undone without adding it to the undo stack or emitting any type of event.

Additionally, since the TextAnimationController currently relies on being able to get the current range using
the session identifier, add a new property to the Writing Tools command to give this information.

* Source/WebCore/editing/CompositeEditCommand.cpp:
(WebCore::EditCommandComposition::unapply):
* Source/WebCore/editing/CompositeEditCommand.h:
* Source/WebCore/editing/WritingToolsCompositionCommand.cpp:
(WebCore::WritingToolsCompositionCommand::WritingToolsCompositionCommand):
(WebCore::WritingToolsCompositionCommand::replaceContentsOfRangeWithFragment):
* Source/WebCore/editing/WritingToolsCompositionCommand.h:
(WebCore::WritingToolsCompositionCommand::currentContextRange const):
* Source/WebCore/page/writing-tools/WritingToolsController.mm:
(WebCore::WritingToolsController::compositionSessionDidReceiveTextWithReplacementRange):
(WebCore::WritingToolsController::contextRangeForSessionWithID const):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WritingTools.mm:
(makeTableAttributedString):
(TEST(WritingTools, CompositionWithTable)):
(TEST(WritingTools, SmartReplyWithInsertedSpace)):

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

f8c257f

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~	✅ 🧪 wincairo-tests
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	~~🧪 api-mac~~	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	~~🧪 api-ios~~	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	~~🧪 mac-AS-debug-wk2~~	~~🧪 gtk-wk2~~
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	~~🧪 api-gtk~~
✅ 🛠 🧪 merge	✅ 🧪 vision-wk2
	~~🛠 tv~~
	~~🛠 tv-sim~~
	~~🛠 watch~~
	~~🛠 watch-sim~~

webkit-early-warning-system · 2024-07-08T14:45:46Z

EWS run on previous version of this PR (hash b18d8a4)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 wincairo-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	⏳ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2
	✅ 🛠 tv
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

Source/WebCore/editing/CompositeEditCommand.h

Tools/TestWebKitAPI/Tests/WebKitCocoa/WritingTools.mm

Source/WebCore/editing/CompositeEditCommand.cpp

webkit-early-warning-system · 2024-07-08T21:54:32Z

EWS run on previous version of this PR (hash 6cd9217)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 wincairo-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	❌ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2
	✅ 🛠 tv
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-07-09T17:01:07Z

EWS run on previous version of this PR (hash 87f2064)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 wincairo-tests
✅ 🧪 webkitperl	❌ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2
	✅ 🛠 tv
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-07-14T02:15:10Z

EWS run on previous version of this PR (hash ab4cc39)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 wincairo-tests
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2
	✅ 🛠 tv
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-07-14T03:15:54Z

EWS run on previous version of this PR (hash 5ce998b)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 wincairo-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2
	✅ 🛠 tv
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-07-15T15:16:22Z

EWS run on previous version of this PR (hash 19404df)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 wincairo-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2
	✅ 🛠 tv
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

Tools/TestWebKitAPI/Tests/WebKitCocoa/WritingTools.mm

webkit-early-warning-system · 2024-07-15T21:07:41Z

EWS run on previous version of this PR (hash 8c548d1)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 wincairo-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2
	✅ 🛠 tv
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-07-16T00:15:23Z

EWS run on previous version of this PR (hash 6d0deb6)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 wincairo-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
	✅ 🧪 vision-wk2
	✅ 🛠 tv
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-08-02T21:43:49Z

EWS run on current version of this PR (hash f8c257f)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~	✅ 🧪 wincairo-tests
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	~~🧪 api-mac~~	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	~~🧪 api-ios~~	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	~~🧪 mac-AS-debug-wk2~~	~~🧪 gtk-wk2~~
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	~~🧪 api-gtk~~
✅ 🛠 🧪 merge	✅ 🧪 vision-wk2
	~~🛠 tv~~
	~~🛠 tv-sim~~
	~~🛠 watch~~
	~~🛠 watch-sim~~

https://bugs.webkit.org/show_bug.cgi?id=276315 rdar://129926389 Reviewed by Aditya Keerthi. When `didReceiveText` is invoked multiple times when replacing text with a table, multiple table elements were being created. This is because when the controller tries to re-create the context range after the first table, it is unable to do so since selections cannot encompass a table or list element. Fix by not trying to re-create the context range at all; instead, just undo the previous replacements, and then the context range will always just be the original range for the current composition. To facilitate this, add a 'silent' option when undo-ing a composition edit command so that a command can be undone without adding it to the undo stack or emitting any type of event. Additionally, since the TextAnimationController currently relies on being able to get the current range using the session identifier, add a new property to the Writing Tools command to give this information. * Source/WebCore/editing/CompositeEditCommand.cpp: (WebCore::EditCommandComposition::unapply): * Source/WebCore/editing/CompositeEditCommand.h: * Source/WebCore/editing/WritingToolsCompositionCommand.cpp: (WebCore::WritingToolsCompositionCommand::WritingToolsCompositionCommand): (WebCore::WritingToolsCompositionCommand::replaceContentsOfRangeWithFragment): * Source/WebCore/editing/WritingToolsCompositionCommand.h: (WebCore::WritingToolsCompositionCommand::currentContextRange const): * Source/WebCore/page/writing-tools/WritingToolsController.mm: (WebCore::WritingToolsController::compositionSessionDidReceiveTextWithReplacementRange): (WebCore::WritingToolsController::contextRangeForSessionWithID const): * Tools/TestWebKitAPI/Tests/WebKitCocoa/WritingTools.mm: (makeTableAttributedString): (TEST(WritingTools, CompositionWithTable)): (TEST(WritingTools, SmartReplyWithInsertedSpace)): Canonical link: https://commits.webkit.org/281780@main

webkit-commit-queue · 2024-08-02T23:39:46Z

Committed 281780@main (292bb3d): https://commits.webkit.org/281780@main

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

rr-codes requested review from cdumez and rniwa as code owners July 8, 2024 14:45

rr-codes self-assigned this Jul 8, 2024

rr-codes added the HTML Editing For bugs in HTML editing support (including designMode and contentEditable). label Jul 8, 2024

rr-codes requested review from aprotyas, hortont424, lilyspiniolas, megangardner, pxlcoder and whsieh July 8, 2024 14:46

pxlcoder reviewed Jul 8, 2024

View reviewed changes

Source/WebCore/editing/CompositeEditCommand.h Outdated Show resolved Hide resolved

Tools/TestWebKitAPI/Tests/WebKitCocoa/WritingTools.mm Outdated Show resolved Hide resolved

rniwa reviewed Jul 8, 2024

View reviewed changes

Source/WebCore/editing/CompositeEditCommand.cpp Outdated Show resolved Hide resolved

rr-codes force-pushed the eng/129926389 branch from b18d8a4 to 6cd9217 Compare July 8, 2024 21:52

rr-codes requested a review from pxlcoder July 8, 2024 21:52

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jul 9, 2024

rr-codes removed the merging-blocked Applied to prevent a change from being merged label Jul 9, 2024

rr-codes force-pushed the eng/129926389 branch from 6cd9217 to 87f2064 Compare July 9, 2024 16:58

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jul 9, 2024

rr-codes removed the merging-blocked Applied to prevent a change from being merged label Jul 14, 2024

rr-codes force-pushed the eng/129926389 branch from 87f2064 to ab4cc39 Compare July 14, 2024 02:15

rr-codes force-pushed the eng/129926389 branch from ab4cc39 to 5ce998b Compare July 14, 2024 03:15

rr-codes force-pushed the eng/129926389 branch from 5ce998b to 19404df Compare July 15, 2024 15:16

pxlcoder reviewed Jul 15, 2024

View reviewed changes

Tools/TestWebKitAPI/Tests/WebKitCocoa/WritingTools.mm Outdated Show resolved Hide resolved

pxlcoder reviewed Jul 15, 2024

View reviewed changes

Tools/TestWebKitAPI/Tests/WebKitCocoa/WritingTools.mm Outdated Show resolved Hide resolved

pxlcoder approved these changes Jul 15, 2024

View reviewed changes

rr-codes force-pushed the eng/129926389 branch from 19404df to 8c548d1 Compare July 15, 2024 21:07

rr-codes force-pushed the eng/129926389 branch from 8c548d1 to 6d0deb6 Compare July 16, 2024 00:15

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jul 16, 2024

rr-codes removed the merging-blocked Applied to prevent a change from being merged label Jul 16, 2024

rr-codes force-pushed the eng/129926389 branch from 6d0deb6 to f8c257f Compare August 2, 2024 21:42

rr-codes added the merge-queue Applied to send a pull request to merge-queue label Aug 2, 2024

webkit-commit-queue force-pushed the eng/129926389 branch from f8c257f to 292bb3d Compare August 2, 2024 23:39

webkit-commit-queue merged commit 292bb3d into WebKit:main Aug 2, 2024

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Aug 2, 2024

[Writing Tools] Make Table: Mail: Generated table has extra borderlines #30564

[Writing Tools] Make Table: Mail: Generated table has extra borderlines #30564

Uh oh!

Conversation

rr-codes commented Jul 8, 2024 • edited by webkit-early-warning-system Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

292bb3d

Uh oh!

webkit-early-warning-system commented Jul 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

webkit-early-warning-system commented Jul 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Jul 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Jul 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Jul 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Jul 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

webkit-early-warning-system commented Jul 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Jul 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Aug 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-commit-queue commented Aug 2, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

rr-codes commented Jul 8, 2024 •

edited by webkit-early-warning-system

Loading

`292bb3d`

webkit-early-warning-system commented Jul 8, 2024 •

edited

Loading

webkit-early-warning-system commented Jul 8, 2024 •

edited

Loading

webkit-early-warning-system commented Jul 9, 2024 •

edited

Loading

webkit-early-warning-system commented Jul 14, 2024 •

edited

Loading

webkit-early-warning-system commented Jul 14, 2024 •

edited

Loading

webkit-early-warning-system commented Jul 15, 2024 •

edited

Loading

webkit-early-warning-system commented Jul 15, 2024 •

edited

Loading

webkit-early-warning-system commented Jul 16, 2024 •

edited

Loading

webkit-early-warning-system commented Aug 2, 2024 •

edited

Loading