Fix toggleMark bug for selections with depth zero

tilgovi commented

2022-09-01 01:28:32 +02:00

(Migrated from github.com)

When a selection range starts at depth zero, allow toggleMark commands
to succeed only if the top node both allows the mark and has inline
content. These restrictions align with the restrictions for the command
to succeed on other selections, fixing an edge case that 5692407 did not
catch.

When a selection range starts at depth zero, allow toggleMark commands to succeed only if the top node both allows the mark and has inline content. These restrictions align with the restrictions for the command to succeed on other selections, fixing an edge case that 5692407 did not catch.

tilgovi commented

2022-09-01 01:29:15 +02:00

(Migrated from github.com)

I think it's also possible that this could be implemented maybe simply by using the parent argument to the traversal function, making it unnecessary to handle the top node in any special way.

I think it's also possible that this could be implemented maybe simply by using the `parent` argument to the traversal function, making it unnecessary to handle the top node in any special way.

tilgovi commented

2022-09-01 01:31:04 +02:00

(Migrated from github.com)

I think it's also possible that this could be implemented maybe simply by using the parent argument to the traversal function, making it unnecessary to handle the top node in any special way.

The way it is already may be better, because it can avoid a level of recursion that the other approach might take.

> I think it's also possible that this could be implemented maybe simply by using the parent argument to the traversal function, making it unnecessary to handle the top node in any special way. The way it is already may be better, because it can avoid a level of recursion that the other approach might take.

tilgovi commented

2022-09-01 01:32:21 +02:00

(Migrated from github.com)

There may also be a related bug here, where $to.depth is zero but $from.depth is not. I don't know if that's realistic for a document with a top node that has inline content or not.

There may also be a related bug here, where `$to.depth` is zero but `$from.depth` is not. I don't know if that's realistic for a document with a top node that has inline content or not.

tilgovi commented

2022-09-01 01:38:53 +02:00

(Migrated from github.com)

In that vein, I pushed an additional commit that I think avoids some other unnecessary recursion. If that's wrong, or you'd prefer it as a separate PR, please let me know.

marijnh commented

2022-09-01 07:48:39 +02:00

(Migrated from github.com)

I've merged the first patch as 9a187ec. The second doesn't seem worthwhile.

I've merged the first patch as 9a187ec. The second doesn't seem worthwhile.

❤️ 1

tilgovi commented

2022-09-01 08:57:19 +02:00

(Migrated from github.com)

Thanks for the quick response!

tilgovi commented

2022-09-08 22:57:32 +02:00

(Migrated from github.com)

If it isn't too much trouble and there's no other planned work that you'd want to do here beforehand, would it be possible to make a release with this change? I'm shipping a local copy of toggleMark to work around this currently.

If it isn't too much trouble and there's no other planned work that you'd want to do here beforehand, would it be possible to make a release with this change? I'm shipping a local copy of `toggleMark` to work around this currently.

marijnh commented

2022-09-08 23:22:11 +02:00

(Migrated from github.com)

No, definitely no trouble. I've tagged 1.3.1

tilgovi commented

2022-09-09 05:39:57 +02:00

(Migrated from github.com)

🙏🏻 Thank you!

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Rows
Columns

Fix toggleMark bug for selections with depth zero #14

Pull request closed