ProseMirror / prosemirror

The ProseMirror WYSIWYM editor
http://prosemirror.net/
MIT License
7.74k stars 336 forks source link

Upgrading prosemirror-model to any version above 1.9.1 breaks removeMark behavior #1083

Open furkan3ayraktar opened 4 years ago

furkan3ayraktar commented 4 years ago

Issue details

If I upgrade prosemirror-model to any version above 1.9.1, I get an exception when tr.removeMark is called with a markType. When the version is downgraded to 1.9.1, no exception thrown and everything works perfectly.

Steps to reproduce

Call tr.removeMark with a markType. If the mark type used in this call manipulates the styles of the DOM element, exception is thrown right away (see first screenshot below). If the mark does not manipulate the styles of DOM element, the removeMark is executed without an exception but it throws an exception when the collab plugin tries to serialize the step via RemoveMarkStep.toJSON (see second screenshot).

ProseMirror version

prosemirror-collab: 1.2.2 prosemirror-commands: 1.1.4 prosemirror-history: 1.1.3 prosemirror-keymap: 1.1.4 prosemirror-model: 1.11.0 prosemirror-state: 1.3.3 prosemirror-transform: 1.2.8 prosemirror-view: 1.15.3

Affected platforms

I tried with the browsers below:

Screenshots / Screencast (Optional)

Stacktrace when a mark which manipulates styles is removed:

image

Stacktrace when a mark is tried to be serialized:

image
marijnh commented 4 years ago

I can't reproduce this (doing let s = view.state.selection; view.dispatch(view.state.tr.removeMark(s.from, s.to, view.state.schema.marks.em)) in the demo when an emphasized bit of code is selected works fine). Are you sure the mark type is from the correct schema? If so, can you produce a script that demonstrates the problem?

furkan3ayraktar commented 4 years ago

Thanks for the quick reply! Here are some snippets from my code.

This is the function where removeMark is called and the exception is thrown:

const toggleHighlights = (editor: EditorView, color?: string): boolean => {
  const { state, dispatch } = editor;
  const { empty, ranges } = state.selection;
  const markType = state.schema.marks.highlight;

  if (empty || !markApplies(state.doc, ranges, markType)) {
    return false;
  }

  if (dispatch) {
    let { tr } = state;

    for (let i = 0; i < ranges.length; i += 1) {
      const { $from, $to } = ranges[i];

      const hasMark = state.doc.rangeHasMark($from.pos, $to.pos, markType);

      if (hasMark) {
        tr = tr.removeMark($from.pos, $to.pos, markType);
      }

      if (color) {
        tr = tr.addMark($from.pos, $to.pos, markType.create({ color }));
      }
    }

    dispatch(tr.scrollIntoView());
  }

  return true;
};

This is the part of schema that defines the highight mark type:

...

const convertColor = (color: string | undefined): string => {
  switch (color) {
    case 'purple':
      return '#cdb2f4';
    case 'teal':
      return '#96f6fa';
    case 'green':
      return '#bdfbb3';
    case 'yellow':
      return '#fff1b8';
    default:
      break;
  }
  return '#ffc9ad';
};

const highlightProps = (attrs: { [key: string]: any }): { [attr: string]: string } => {
  if (attrs.color) {
    return {
      style: `background-color: ${convertColor(attrs.color)};`,
    };
  }

  return {};
};

export const schema = new Schema({
  ...

  marks: {
    ...

    highlight: {
      attrs: { color: { default: 'red' } },
      toDOM: (mark: Mark): DOMOutputSpecArray => ['span', highlightProps(mark.attrs)],
    },

    ...
  }
});

Let me know if this helps or if you need more information.

marijnh commented 4 years ago

I don't really want a bunch of snippets that I have to piece together in the hope that they'll help reproduce the issue. I'd need a minimal self-contained script that shows the problem.

furkan3ayraktar commented 4 years ago

I understand that, I'll examine more the issue and see if I can create a minimal setup to reproduce the issue.