Skip to content
This repository has been archived by the owner on Sep 27, 2024. It is now read-only.

Loading HTML containing @room creates nested links and eventually crashes #855

Open
jmartinesp opened this issue Oct 24, 2023 · 1 comment

Comments

@jmartinesp
Copy link
Contributor

I could only test this on Android, for now, but the steps I took were:

  1. Type @room in the example editor, get a pill.
  2. Switch the light/dark theme (this causes re-creation, so the HTML is fed again to the RTE if I'm not mistaken).
  3. Switch it again.
  4. Switch it again. The app will crash.

This can also be reproduced by rotating the phone, anything that causes an Activity (and views) re-creation.

In the logs, I see, for each state:

  1. @room |
  2. <a data-mention-type="at-room" href="#" contenteditable="false">@room</a>&nbsp;|
  3. <a data-mention-type="at-room" contenteditable="false" href="#"><a data-mention-type="at-room" href="#" contenteditable="false">@room</a>|</a>

And finally:

io....nt.wysiwyg.compose  D  <a data-mention-type="at-room" contenteditable="false" href="#"><a data-mention-type="at-room" href="#" contenteditable="false">@room</a>|</a>
io....nt.wysiwyg.compose  E  uniffi.wysiwyg_composer.DomCreationException$HtmlParseException: 
io....nt.wysiwyg.compose  E  	at uniffi.wysiwyg_composer.FfiConverterTypeDomCreationError.read(wysiwyg_composer.kt:2113)
io....nt.wysiwyg.compose  E  	at uniffi.wysiwyg_composer.FfiConverterTypeDomCreationError.read(wysiwyg_composer.kt:2110)
io....nt.wysiwyg.compose  E  	at uniffi.wysiwyg_composer.FfiConverter$DefaultImpls.liftFromRustBuffer(wysiwyg_composer.kt:170)
io....nt.wysiwyg.compose  E  	at uniffi.wysiwyg_composer.FfiConverterRustBuffer$DefaultImpls.liftFromRustBuffer(wysiwyg_composer.kt:182)
io....nt.wysiwyg.compose  E  	at uniffi.wysiwyg_composer.FfiConverterTypeDomCreationError.liftFromRustBuffer(wysiwyg_composer.kt:2110)
io....nt.wysiwyg.compose  E  	at uniffi.wysiwyg_composer.FfiConverterTypeDomCreationError.liftFromRustBuffer(wysiwyg_composer.kt:2110)
io....nt.wysiwyg.compose  E  	at uniffi.wysiwyg_composer.FfiConverterRustBuffer$DefaultImpls.lift(wysiwyg_composer.kt:183)
io....nt.wysiwyg.compose  E  	at uniffi.wysiwyg_composer.FfiConverterTypeDomCreationError.lift(wysiwyg_composer.kt:2110)
io....nt.wysiwyg.compose  E  	at uniffi.wysiwyg_composer.FfiConverterTypeDomCreationError.lift(wysiwyg_composer.kt:2110)
io....nt.wysiwyg.compose  E  	at uniffi.wysiwyg_composer.DomCreationException$ErrorHandler.lift(wysiwyg_composer.kt:2106)
io....nt.wysiwyg.compose  E  	at uniffi.wysiwyg_composer.DomCreationException$ErrorHandler.lift(wysiwyg_composer.kt:2105)
io....nt.wysiwyg.compose  E  	at uniffi.wysiwyg_composer.Wysiwyg_composerKt.checkCallStatus(wysiwyg_composer.kt:235)
io....nt.wysiwyg.compose  E  	at uniffi.wysiwyg_composer.Wysiwyg_composerKt.access$checkCallStatus(wysiwyg_composer.kt:1)
io....nt.wysiwyg.compose  E  	at uniffi.wysiwyg_composer.ComposerModel.setContentFromHtml(wysiwyg_composer.kt:3192)
io....nt.wysiwyg.compose  E  	at io.element.android.wysiwyg.internal.viewmodel.EditorViewModel.processInput(EditorViewModel.kt:104)
io....nt.wysiwyg.compose  E  	at io.element.android.wysiwyg.EditorEditText.setHtml(EditorEditText.kt:423)
io....nt.wysiwyg.compose  E  	at io.element.android.wysiwyg.compose.RichTextEditorKt$RealEditor$1.invoke(RichTextEditor.kt:119)
io....nt.wysiwyg.compose  E  	at io.element.android.wysiwyg.compose.RichTextEditorKt$RealEditor$1.invoke(RichTextEditor.kt:82)
io....nt.wysiwyg.compose  E  	at androidx.compose.ui.viewinterop.ViewFactoryHolder.<init>(AndroidView.android.kt:337)
io....nt.wysiwyg.compose  E  	at androidx.compose.ui.viewinterop.AndroidView_androidKt$createAndroidViewNodeFactory$1.invoke(AndroidView.android.kt:271)
io....nt.wysiwyg.compose  E  	at androidx.compose.ui.viewinterop.AndroidView_androidKt$createAndroidViewNodeFactory$1.invoke(AndroidView.android.kt:270)
io....nt.wysiwyg.compose  E  	at androidx.compose.ui.viewinterop.AndroidView_androidKt$AndroidView$$inlined$ComposeNode$1.invoke(Composables.kt:254)
io....nt.wysiwyg.compose  E  	at androidx.compose.runtime.ComposerImpl$createNode$2.invoke(Composer.kt:1610)
io....nt.wysiwyg.compose  E  	at androidx.compose.runtime.ComposerImpl$createNode$2.invoke(Composer.kt:1608)
io....nt.wysiwyg.compose  E  	at androidx.compose.runtime.ComposerImpl$recordInsert$2.invoke(Composer.kt:3546)
io....nt.wysiwyg.compose  E  	at androidx.compose.runtime.ComposerImpl$recordInsert$2.invoke(Composer.kt:3543)
io....nt.wysiwyg.compose  E  	at androidx.compose.runtime.CompositionImpl.applyChangesInLocked(Composition.kt:818)
io....nt.wysiwyg.compose  E  	at androidx.compose.runtime.CompositionImpl.applyChanges(Composition.kt:849)
io....nt.wysiwyg.compose  E  	at androidx.compose.runtime.Recomposer.composeInitial$runtime_release(Recomposer.kt:1041)
io....nt.wysiwyg.compose  E  	at androidx.compose.runtime.CompositionImpl.setContent(Composition.kt:520)
io....nt.wysiwyg.compose  E  	at androidx.compose.ui.platform.WrappedComposition$setContent$1.invoke(Wrapper.android.kt:142)
io....nt.wysiwyg.compose  E  	at androidx.compose.ui.platform.WrappedComposition$setContent$1.invoke(Wrapper.android.kt:133)
io....nt.wysiwyg.compose  E  	at androidx.compose.ui.platform.AndroidComposeView.setOnViewTreeOwnersAvailable(AndroidComposeView.android.kt:1191)
io....nt.wysiwyg.compose  E  	at androidx.compose.ui.platform.WrappedComposition.setContent(Wrapper.android.kt:133)
io....nt.wysiwyg.compose  E  	at androidx.compose.ui.platform.WrappedComposition.onStateChanged(Wrapper.android.kt:183)
io....nt.wysiwyg.compose  E  	at androidx.lifecycle.LifecycleRegistry$ObserverWithState.dispatchEvent(LifecycleRegistry.kt:314)
io....nt.wysiwyg.compose  E  	at androidx.lifecycle.LifecycleRegistry.addObserver(LifecycleRegistry.kt:192)
io....nt.wysiwyg.compose  E  	at androidx.compose.ui.platform.WrappedComposition$setContent$1.invoke(Wrapper.android.kt:140)
io....nt.wysiwyg.compose  E  	at androidx.compose.ui.platform.WrappedComposition$setContent$1.invoke(Wrapper.android.kt:133)
io....nt.wysiwyg.compose  E  	at androidx.compose.ui.platform.AndroidComposeView.onAttachedToWindow(AndroidComposeView.android.kt:1266)
io....nt.wysiwyg.compose  E  	at android.view.View.dispatchAttachedToWindow(View.java:20626)
io....nt.wysiwyg.compose  E  	at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:3514)
io....nt.wysiwyg.compose  E  	at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:3521)
io....nt.wysiwyg.compose  I  uid=10522(io.element.wysiwyg.compose) identical 2 lines
io....nt.wysiwyg.compose  E  	at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:3521)
io....nt.wysiwyg.compose  E  	at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:2726)
io....nt.wysiwyg.compose  E  	at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:2206)
io....nt.wysiwyg.compose  E  	at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:8763)
io....nt.wysiwyg.compose  E  	at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1352)
io....nt.wysiwyg.compose  E  	at android.view.Choreographer.doCallbacks(Choreographer.java:1149)
io....nt.wysiwyg.compose  E  	at android.view.Choreographer.doFrame(Choreographer.java:1049)
io....nt.wysiwyg.compose  E  	at android.view.Choreographer$FrameHandler.handleMessage(Choreographer.java:1275)
io....nt.wysiwyg.compose  E  	at android.os.Handler.dispatchMessage(Handler.java:106)
io....nt.wysiwyg.compose  E  	at android.os.Looper.loop(Looper.java:233)
io....nt.wysiwyg.compose  E  	at android.app.ActivityThread.main(ActivityThread.java:8068)

2023-10-24 17:24:55.256 MainActivity$onCreate     io....nt.wysiwyg.compose  E  	at java.lang.reflect.Method.invoke(Native Method)
io....nt.wysiwyg.compose  E  	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:631)
io....nt.wysiwyg.compose  E  	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:978)
io....nt.wysiwyg.compose  D  Shutting down VM
@jmartinesp jmartinesp changed the title Loading HTML containing @room creates nested links Loading HTML containing @room creates nested links and eventually crashes Oct 24, 2023
@aringenbach
Copy link
Contributor

Seems like it is this code:

fn convert_text<S: UnicodeString>(
    text: &str,
    node: &mut ContainerNode<S>,
    is_inside_code_block: bool,
) {
    if is_inside_code_block {
        let text_nodes: Vec<_> = text.split('\n').collect();
        let text_nodes_len = text_nodes.len();
        for (i, str) in text_nodes.into_iter().enumerate() {
            let is_nbsp = str == "\u{A0}" || str == "&nbsp;";
            if !str.is_empty() && !is_nbsp {
                let text_node = DomNode::new_text(str.into());
                node.append_child(text_node);
            }
            if i + 1 < text_nodes_len {
                node.append_child(DomNode::new_line_break());
            }
        }
    } else {
        let contents = text;
        let is_nbsp = contents == "\u{A0}" || contents == "&nbsp;";
        if is_nbsp {
            return;
        }

        // Trim any surrounding indentation
        let surrounding_indent =
            Regex::new(r"^(\s*\n\s*)+|(\s*\n\s*)+$").unwrap();
        let contents = &surrounding_indent.replace_all(contents, "");

        // Replace any internal indentation with a single space
        let internal_indent = Regex::new(r"s*\n\s*").unwrap();
        let contents = &internal_indent.replace_all(contents, " ");

        for (i, part) in contents.split("@room").enumerate() {
            if i > 0 {
                node.append_child(DomNode::Mention(
                    DomNode::new_at_room_mention(vec![]),
                ));
            }
            if !part.is_empty() {
                node.append_child(DomNode::new_text(part.into()));
            }
        }
    }
}

We need to do that last part only if we are not parsing an already created mention.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants