Fix `getBookmark` bug

RichieAHB commented

2018-07-02 16:33:21 +02:00

(Migrated from github.com)

I've been banging my head against 'can't read property inlineContent of undefined' for a while and I think I may have found the culprit. It seems Selection.between expects a ResolvedPos but here getBookmark was passing the number of a pos instead. This was being fired by prosemirror-history. I think this slipped under the radar because it's looks like it only fires in certain history grouping scenarios cases but even then I'm surprised this hasn't surfaced earlier so am a bit confused! Either way, it fixes a couple of tests I've been struggling with.

I've been banging my head against 'can't read property inlineContent of undefined' for a while and I think I may have found the culprit. It seems `Selection.between` expects a `ResolvedPos` but here `getBookmark` was passing the number of a pos instead. This was being fired by `prosemirror-history`. I think this slipped under the radar because it's looks like it only fires in certain history grouping scenarios cases but even then I'm surprised this hasn't surfaced earlier so am a bit confused! Either way, it fixes a couple of tests I've been struggling with.

marijnh commented

2018-07-02 16:41:49 +02:00

(Migrated from github.com)

I guess this only gets called when you have a custom selection class that doesn't override this method (the built in ones do override it), and that might explain how it sat there for so long. But yeah, this is the kind of thing where TypeScript would help.

RichieAHB commented

2018-07-02 17:03:49 +02:00

(Migrated from github.com)

Makes sense, I was just using a raw Selection class there which probably could have avoided that bug in hindsight.

Makes sense, I was just using a raw `Selection` class there which probably could have avoided that bug in hindsight.

marijnh commented

2018-07-02 17:07:45 +02:00

(Migrated from github.com)

You don't want to ever use a raw Selection instance. It'll be missing methods.

You don't want to ever use a raw `Selection` instance. It'll be missing methods.

RichieAHB commented

2018-07-02 19:51:34 +02:00

(Migrated from github.com)

Only really for a small edge case in text ... is there any reason it’s
exported from the package if it’s not usable?
On Mon, 2 Jul 2018 at 16:07, Marijn Haverbeke notifications@github.com
wrote:

You don't want to ever use a raw Selection instance. It'll be missing
methods.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/ProseMirror/prosemirror-state/pull/12#issuecomment-401836871,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABk128nz5jhI7sslukiTlhJMFRAVdyQVks5uCjdCgaJpZM4U_bPJ
.

Only really for a small edge case in text ... is there any reason it’s exported from the package if it’s not usable? On Mon, 2 Jul 2018 at 16:07, Marijn Haverbeke <notifications@github.com> wrote: > You don't want to ever use a raw Selection instance. It'll be missing > methods. > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <https://github.com/ProseMirror/prosemirror-state/pull/12#issuecomment-401836871>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/ABk128nz5jhI7sslukiTlhJMFRAVdyQVks5uCjdCgaJpZM4U_bPJ> > . >

marijnh commented

2018-07-02 22:34:31 +02:00

(Migrated from github.com)

is there any reason it’s exported from the package if it’s not usable?

It can be used as superclass for custom selection classes.

> is there any reason it’s exported from the package if it’s not usable? It can be used as superclass for custom selection classes.

👍 1

Rows
Columns

Fix getBookmark bug #12

Fix `getBookmark` bug #12