<a class="header-button" href="https://bugzilla.mozilla.org/home" title="Go to home page"> Bugzilla

Comment 4

•

3 years ago

Although I still don't find the cause in the site's script, I need to fix bug 1807829 first due to user facing web-compat issue in a major app. I'll be back after writing a patch for it.

Flags: needinfo?(masayuki)

Comment 5

•

3 years ago

Hello Dennis, would your team be able to help us investigate what's up with the Reddit's code, with the preference "editor.join_split_direction.compatible_with_the_other_browsers flipping to true? The diagnosis information would help us to decide our next step to enable the change. Thank you!

Flags: needinfo?(dschubert)

Raul Bucata

Comment 6

•

3 years ago

We appreciate your report. I was able to reproduce the issue.

Tested with:

Browser / Version: Firefox Release 109.0 (64-bit)/ Firefox Nightly 111.0a1 (2023-01-31) (64-bit) / Chrome Version 109.0.5414.120 (Official Build) (64-bit)
Operating System: Windows 10 PRO x64

Notes:

Reproducible regardless of the status of ETP.
Reproducible on the latest build of Firefox Nightly and Release.
Works as expected using Chrome.

Comment 7

•

2 years ago

(In reply to Hsin-Yi Tsai (she/her) [:hsinyi] from comment #5)

Hello Dennis, would your team be able to help us investigate what's up with the Reddit's code, [...]

Looking into this, I noticed that this only affects text that has been copied from Reddits editor. Pasting text in that's copied from elsewhere, for example from this bug or my text editor, works fine. This almost leads me to believe that the issue here is copying, not pasting. It also only affects the "fancy pants editor" within the comment section - trying to compose a new post using the same editor very much doesn't show this behavior, which is odd.

Reddit is using Draft.js, but this doesn't reproduce in a default Draft.js instance, so it's something in their custom editor code on top of Draft.js. Looking at their logic, they want to restore some state that's previously been saved. In Chrome, they store the state in an event listener they internally call onBeforeInput - but confusingly, this is not actually an event handler for beforeinput, and it's instead for the textinput event, which is a Chrome-ism we don't support (see bug 903746), and that they probably won't remove anytime soon.

Luckily, I'm not surprised by this - we know that draft.js depends on the textinput event, and we ship an intervention to Facebook and Twitter for exactly this reason (where we just map the textinput event to the beforeinput event to work around all kinds of issues). Unfortunately, however, adding this shim to Reddit does not fix the editor.

I can sometimes see the handler function from node_modules/draft-js/lib/editOnBeforeInput.js handler firing in Firefox, so it clearly works sometimes. However, when pasting something, this function fires in Chrome, but doesn't in Firefox. This is odd, since the call should just be an event listener to beforeinput. Looking at Chrome, I'm exceptionally confused. Even though they're somehow ending up in the onBeforeInput event handler, the TextEvent object passed into the handler has TextEvent.type="textInput".

This doesn't make sense - as beforeinput is an InputEvent, not a TextEvent. Moreover, while in an isolated testcase, chrome fires both beforeinput and textInput in that order - the textInput event has data="", while on Reddit, the event has the to-be-pasted text as data. So it looks like their code is intercepting pastes,, and is dispatching an textInput event for the pasted text instead.

The reddit shource has a paste event handler from node_modules/draft-js/lib/editOnPaste.js - but that one doesn't fire in Chrome or Firefox. However, before I do that: Tom, you looked into Draft.js code before. Have you ever seen them re-map paste events like I described above? If you don't know what's going on here off the top of your head, I can dig deeper into that, but I figured it's worth asking you first... :)

Flags: needinfo?(dschubert) → needinfo?(twisniewski)

Comment 8

•

2 years ago

re-ni?'ing myself - I did not mean to drop that.

Flags: needinfo?(dschubert)

Comment 9

•

2 years ago

Okay, so when I enable our draft.js intervention on www.reddit.com and try to paste, I see errors in the console when I paste, so they likely rely on textInput there. But it's hard to make sense of what's going on, because their source maps are broken, and switching to minified code makes their framework refuse to spit out the actual error it caught instead of a cryptic "enable source maps to get the actual message" message.

But after a little digging, I see this code being reached:

          switch (e) {
            case 'compositionend':
              return xe(t);
            case 'keypress':
              return 32 !== t.which ? null : (je = !0, ge);
            case 'textInput':
              return (e = t.data) === ge && je ? null : e;
            default:
              return null
          }

And if I disable the draft.js intervention, it never hits this code at all. So they only seem to handle textInput events for pasting, rather than the paste event. And this code seems to corroborate that:

          if (Se) return 'compositionend' === e || !be && Ee(e, t) ? (e = se(), ie = oe = re = null, Se = !1, e) : null;
          switch (e) {
            case 'paste':
              return null;
            case 'keypress':

But that's not the only problem, because even when it hits that event handler for textInput, the data of the vent is null. So yeah, it's likely also something related to their copying code. I'll see if I can't figure that out.

Comment 10

•

2 years ago

Ahh, no, I was wrong, they're not preventing default on the copy event, and the system clipboard does get whatever I copy.

The problem is that beforeInput doesn't set data at all, it just sets dataTransfer. So if I shim the InputEvent.prototype.data to just return the plain text of the event's dataTransfer, by adding this in our existing draft.js intervention, the pasting starts to almost work:

const proto = InputEvent.wrappedJSObject.prototype;
Object.defineProperty(proto, "data", {
  get: exportFunction(function() {
    return this?.dataTransfer?.getData("text/plain");
  }, window),
});

But it's still acting in strange ways for text copied off the page, despite the values of ge and e.data in the textInput function being the same in both cases. It seems like the cursor loses its position in the text box? Either way, it seems unrelated to this code I've been looking at, so there's still more to the story here.

Comment 11

•

2 years ago

I've poked around a fair bit more here, and unfortunately can't quite make heads or tails of what their code is doing. It seems to rely on textinput being a separate event from beforeinput, and race conditions in their script are otherwise triggered which cause the editor to de-sync and lose track of where the cursor is, not allow me to ctrl-a, and other bizarre behavior.

I also tried testing with editor.join_split_direction.compatible_with_the_other_browsers to true, and while I could at least paste text with our draft.fs intervention active, it still de-syncs sometimes and I can't coherently delete any pasted text (whether or not it's HTML or forced plain-text as my comment 10 change enforces).

Flags: needinfo?(twisniewski)

Comment 12

•

2 years ago

Thank you very much, Thomas! Do you have a plan to enable draft.js intervention in Reddit?

Flags: needinfo?(twisniewski)

Comment 13

•

2 years ago

If Dennis and I can find a way to make the intervention work well on Reddit, then yes, I would be happy to enable it. But so far I'll confess that I'm not optimistic we'll be able to do it. So far my progress still gives results so buggy as to feel pointless.

Flags: needinfo?(twisniewski)

Comment 14

•

2 years ago

Okay, then, I think that we should contact Reddit to fix it in their side because the root cause is in their code.

hsinyi: WDYT?

Flags: needinfo?(htsai)

Comment 15

•

2 years ago

We haven't had too much success reaching out to Reddit in the past, so I'm not immediately positive about that.

Tom's diagnosis has given me some more clues, I'll try to dig into it again and see if I can find out something else. If not, I'll send them a mail.

Comment 16

•

2 years ago

•

Edited

I wanted to go back to an earlier observation: When I paste plain text into the editor, with the pref in question set to true, then everything appears to be fine. But as soon as I paste something non-plain-text (i.e. stuff copied from the Reddit editor, or some random text on a random website, ...) the editor completely breaks. It's not just that the paste fails, it's that the entire editor state gets messed up. Just now, for example, I pasted a word copied from my previous comment, and the editor broke so badly, I couldn't even press ctrl-A to select everything anymore.

I'm not to keen on trying to build an isolated version of Reddit's setup, as it's probably almost impossible to do so. Not only are they on a very old React version (16.9.0, from August 2019), they also appeared to made custom changes to Draft.js, that would be incredibly hard to extract... Also, looking at their source, I'm also not sure if they're using the paste event at all, or if they just rely on input events to see what's been pasted. As both Tom and me noted, there is a listener for paste, but to be honest, I have no clue where that comes from and what it does. I could probably figure that out, but it's going to take a bit more time than I'm comfortable.

Because we know that contenteditable is a bit of a mess, I wanted to figure out potential difference that could be relevant here. Reddit's editor is managing paragraphs as inidividual <div> elements (actually <div><div>content</div></div>, but I don't think that's relevant here). So a really minified version of Reddits editor, without all the fancy JavaScript, is

data:text/html,<div id="ce" contenteditable="true"><div>aaa</div><div>bbb</div><div>ccc</div></div>

To #ce, I have attached logging event listeners for paste, beforeinput, input, and textInput. To test, I copied the word "something" from my previous comment, and pasted that behind "bbb". This paste is something that would break the Reddit editor.

The paste event is pretty useless, there's nothing really useful in here as we're not pasting a file. ClipboardEvent.clipboardData.items is empty in both Chrome and Firefox.

The beforeinput event has two entries in InputEvent.dataTransfer.items, which are about what you'd expect:

[
  {
    "type": "text/html",
    "data": "<html><head><meta http-equiv=\"content-type\" content=\"text/html; charset=utf-8\"></head><body>something</body></html>"
  },
  {
    "type": "text/plain",
    "data": "something"
  }
]

Which is consistent in both browsers, so I don't think that's causing breakage. A first notable difference is the input event. In Firefox, InputEvent.dataTransfer is set to the clipboard data, matching to the beforeinput event. In Chrome, InputEvent.dataTransfer is null. Both browsers have InputEvent.inputType == "insertFromPaste", and the spec is pretty clear that dataTransfer should be populated, so that smells like a Chrome bug. This might not be relevant, but if Reddit's code is making naive assumptions and uses the presence of dataTransfer as a behavioral switch, this might cause breakage.

Another thing I noticed in Reddit's code is that they sometimes iterate over the editor's .childNodes. It might be that they don't even listen to the clipboard data in any event, but that they instead observe how the editor state changed after the input is done (again, this is 95% crystal balling and 5% knowledge, at best). Using the same testcase and STR as above (pasting a copied "something" from the previous comment), I noticed a difference in document.getElementById("ce").childNodes[1].childNodes (i.e. the childNodes of the bbb row where I pasted "something").

In Chrome, that div has one child node, which contains bbbsomething. However, in Firefox, there's now actually two childNodes, one #text "bbb", and one #text "something". This feels noteworthy, because when I paste a plain-text "something" (in this case copied from my terminal), Firefox, too, only has #text "bbbsomething". Since pasting plain-text works on Reddit, but posting the Bugzilla-copied-"something" does break on Reddit, this extra childNode might be part of the breakage. Before I dig into the contenteditable story myself to figure out why that is and what's going on here, I'll just redirect this question to Masayuki: he knows contenteditable stuff much better than I do. Do you know what is happening here and if this is something we could address?

I will note, however, that the two-text-nodes difference is completely independent of editor.join_split_direction.compatible_with_the_other_browsers. It is, however, possible that the change behind that pref makes Firefox compatible enough so that we're now running into this breakage, instead of something else.

Also, just for the record: we've emailed Reddit about their broken editor in October 2021 and May 2022, to no avail. However, earlier today, I've been told that as part of ongoing WebCompat-outreach-improvement work, it's quite possible we might get a fresh set of contacts "soon". I will coordinate with them if that's a possibility, but don't hold your breaths on that one.

Flags: needinfo?(dschubert) → needinfo?(masayuki)

Comment 17

•

2 years ago

It turns out that more discussion on-going. Clearing the NI from comment 14 for now.

Flags: needinfo?(htsai)

Comment 18

•

2 years ago

Sorry for the delay.

Oddly, I cannot reproduce this bug today (although the pasting behavior is still odd). Is the intervention is enabled in Reddit?

When I test with:

<div class="notranslate public-DraftEditor-content" role="textbox" spellcheck="true" style="outline: none; user-select: text; white-space: pre-wrap; overflow-wrap: break-word;" contenteditable="true">
  <div data-contents="true">
    <div data-offset-key="86c51f_initial-0-0" class="_3LuG0YVLLHE2azRNVaKz7O">
      <div class="" data-block="true" data-editor="86c51f" data-offset-key="86c51f_initial-0-0">
        <div data-offset-key="86c51f_initial-0-0" class="public-DraftStyleDefault-block public-DraftStyleDefault-ltr">
          <span data-offset-key="86c51f_initial-0-0"><span data-text="true">abc def ghi</span></span>
        </div>
      </div>
    </div>
  </div>
</div>

and copy "def", then, click right of the line and paste it, I got:

<div class="notranslate public-DraftEditor-content" role="textbox" spellcheck="true" style="outline: none; user-select: text; white-space: pre-wrap; overflow-wrap: break-word;" contenteditable="true">
  <div data-contents="true">
    <div data-offset-key="86c51f_initial-0-0" class="_3LuG0YVLLHE2azRNVaKz7O">
      <div class="" data-block="true" data-editor="86c51f" data-offset-key="86c51f_initial-0-0">
        <div data-offset-key="86c51f_initial-0-0" class="public-DraftStyleDefault-block public-DraftStyleDefault-ltr">
          <span data-offset-key="86c51f_initial-0-0"><span data-text="true">abc def ghidef</span></span>
        </div>
      </div>
      <div class="" data-block="true" data-editor="86c51f" data-offset-key="75mk4-0-0">
        <div data-offset-key="75mk4-0-0" class="public-DraftStyleDefault-block public-DraftStyleDefault-ltr">
          <span data-offset-key="75mk4-0-0"><br data-text="true"></span>
        </div>
      </div>
    </div>
  </div>
</div>

When I click right of the line, I got following stacks of Selection API.

[Child 57664: Main Thread]: I/SelectionAPI 1b24c36f7e0 Selection::CollapseInLimiter(aPoint={ mParent=000001B215413380 (#text.span.span.div.div.div.div.div[contenteditable="true"].div.div.div.div.div.div.div.div.div.div.div.div['t1_jagg4ti'].div.div.div.div.div.div.div.div.div.div.div.div.div.div['AppRouter-main-content'].div.div['SHORTCUT_FOCUSABLE_DIV'].div.div['2x-container'].body.html.#document, Length()=11), mRef=0000000000000000, mOffset=11 })
[Child 57664: Main Thread]: D/SelectionAPI
#01: mozilla::dom::Selection::CollapseInLimiter (M:\src\dom\base\Selection.cpp:2207)
#02: nsFrameSelection::TakeFocus (M:\src\layout\generic\nsFrameSelection.cpp:1417)
#03: nsFrameSelection::HandleClick (M:\src\layout\generic\nsFrameSelection.cpp:1246)
#04: nsIFrame::MoveCaretToEventPoint (M:\src\layout\generic\nsIFrame.cpp:4925)
#05: nsIFrame::HandleEvent (M:\src\layout\generic\nsIFrame.cpp:4500)
#06: mozilla::EventTargetChainItem::HandleEventTargetChain (M:\src\dom\events\EventDispatcher.cpp:619)
#07: mozilla::EventDispatcher::Dispatch (M:\src\dom\events\EventDispatcher.cpp:1122)
#08: mozilla::PresShell::EventHandler::DispatchEventToDOM (M:\src\layout\base\PresShell.cpp:8683)
[Child 57664: Main Thread]: I/SelectionAPI 1b24c36f7e0 Selection::SetAncestorLimiter(aLimiter=nullptr)
[Child 57664: Main Thread]: D/SelectionAPI
#01: mozilla::dom::Selection::SetAncestorLimiter (M:\src\dom\base\Selection.cpp:1823)
#02: mozilla::EditorBase::FinalizeSelection (M:\src\editor\libeditor\EditorBase.cpp:5241)
#03: mozilla::HTMLEditor::OnBlur (M:\src\editor\libeditor\HTMLEditor.cpp:777)
#04: mozilla::EditorEventListener::HandleEvent (M:\src\editor\libeditor\EditorEventListener.cpp:456)
#05: mozilla::HTMLEditorEventListener::HandleEvent (M:\src\editor\libeditor\HTMLEditorEventListener.cpp:102)
#06: mozilla::EventListenerManager::HandleEventSubType (M:\src\dom\events\EventListenerManager.cpp:1313)
#07: mozilla::EventListenerManager::HandleEventInternal (M:\src\dom\events\EventListenerManager.cpp:1506)
#08: mozilla::EventTargetChainItem::HandleEvent (M:\src\dom\events\EventDispatcher.cpp:350)
[Child 57664: Main Thread]: I/SelectionAPI 1b24c36f7e0 Selection::SetAncestorLimiter(aLimiter=div[contenteditable="true"].div.div.div.div.div.div.div.div.div.div.div.div['t1_jagg4ti'].div.div.div.div.div.div.div.div.div.div.div.div.div.div['AppRouter-main-content'].div.div['SHORTCUT_FOCUSABLE_DIV'].div.div['2x-container'].body.html.#document)
[Child 57664: Main Thread]: D/SelectionAPI
#01: mozilla::dom::Selection::SetAncestorLimiter (M:\src\dom\base\Selection.cpp:1823)
#02: mozilla::HTMLEditor::InitializeSelectionAncestorLimit (M:\src\editor\libeditor\HTMLEditor.cpp:971)
#03: mozilla::EditorBase::InitializeSelection (M:\src\editor\libeditor\EditorBase.cpp:5195)
#04: mozilla::EditorBase::OnFocus (M:\src\editor\libeditor\EditorBase.cpp:5576)
#05: mozilla::HTMLEditor::OnFocus (M:\src\editor\libeditor\HTMLEditor.cpp:752)
#06: mozilla::EditorEventListener::Focus (M:\src\editor\libeditor\EditorEventListener.cpp:1121)
#07: mozilla::EditorEventListener::HandleEvent (M:\src\editor\libeditor\EditorEventListener.cpp:445)
#08: mozilla::HTMLEditorEventListener::HandleEvent (M:\src\editor\libeditor\HTMLEditorEventListener.cpp:102)

And when I paste it with Ctrl + p, I got:

[Child 57664: Main Thread]: I/SelectionAPI 1b24c36f7e0 nsRange::DoSetRange(aStartBoundary=aEndBoundary={ mParent=000001B21540F3A0 (span.div.div.div.div.div[contenteditable="true"].div.div.div.div.div.div.div.div.div.div.div.div['t1_jagg4ti'].div.div.div.div.div.div.div.div.div.div.div.div.div.div['AppRouter-main-content'].div.div['SHORTCUT_FOCUSABLE_DIV'].div.div['2x-container'].body.html.#document, Length()=0), mRef=0000000000000000, mOffset=0 }, aNotInsertedYet=false)      
[Child 57664: Main Thread]: D/SelectionAPI
#01: nsRange::DoSetRange<nsINode *,nsIContent *,nsINode *,nsIContent *> (M:\src\dom\base\nsRange.cpp:1053)
#02: nsRange::ContentRemoved (M:\src\dom\base\nsRange.cpp:718)
#03: mozilla::dom::MutationObservers::NotifyContentRemoved (M:\src\dom\base\MutationObservers.cpp:197)
#04: nsINode::RemoveChildNode (M:\src\dom\base\nsINode.cpp:2212)
#05: nsINode::RemoveChild (M:\src\dom\base\nsINode.cpp:858)
#06: mozilla::dom::Node_Binding::removeChild (M:\fx64-dbg\dom\bindings\NodeBinding.cpp:1138)
#07: mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy,mozilla::dom::binding_detail::ThrowExceptions> (M:\src\dom\bindings\BindingUtils.cpp:3320)
#08: ??? (???:???)
[Child 57664: Main Thread]: I/SelectionAPI 1b24c36f7e0 Selection::RemoveAllRanges()
[Child 57664: Main Thread]: D/SelectionAPI
#01: mozilla::dom::Selection::RemoveAllRanges (M:\src\dom\base\Selection.cpp:1966)
#02: mozilla::dom::Selection_Binding::removeAllRanges (M:\fx64-dbg\dom\bindings\SelectionBinding.cpp:467)
#03: mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy,mozilla::dom::binding_detail::ThrowExceptions> (M:\src\dom\bindings\BindingUtils.cpp:3320)
#04: CallJSNative (M:\src\js\src\vm\Interpreter.cpp:459)
#05: js::InternalCallOrConstruct (M:\src\js\src\vm\Interpreter.cpp:547)
#06: Interpret (M:\src\js\src\vm\Interpreter.cpp:3362)
#07: js::RunScript (M:\src\js\src\vm\Interpreter.cpp:431)
#08: js::InternalCallOrConstruct (M:\src\js\src\vm\Interpreter.cpp:579)
[Child 57664: Main Thread]: I/SelectionAPI 1b24c36f7e0 Selection::AddRangeJS(aRange={ mStart=mEnd={ mParent=000001B2154ACF70 (span.div.div.div.div.div[contenteditable="true"].div.div.div.div.div.div.div.div.div.div.div.div['t1_jagg4ti'].div.div.div.div.div.div.div.div.div.div.div.div.div.div['AppRouter-main-content'].div.div['SHORTCUT_FOCUSABLE_DIV'].div.div['2x-container'].body.html.#document, Length()=1), mRef=0000000000000000, mOffset=0 }, mIsGenerated=false, mCalledByJS=true, mIsDynamicRange=true })
[Child 57664: Main Thread]: D/SelectionAPI
#01: mozilla::dom::Selection::AddRangeJS (M:\src\dom\base\Selection.cpp:1996)
#02: mozilla::dom::Selection_Binding::addRange (M:\fx64-dbg\dom\bindings\SelectionBinding.cpp:371)
#03: mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy,mozilla::dom::binding_detail::ThrowExceptions> (M:\src\dom\bindings\BindingUtils.cpp:3320)
#04: CallJSNative (M:\src\js\src\vm\Interpreter.cpp:459)
#05: js::InternalCallOrConstruct (M:\src\js\src\vm\Interpreter.cpp:547)
#06: Interpret (M:\src\js\src\vm\Interpreter.cpp:3362)
#07: js::RunScript (M:\src\js\src\vm\Interpreter.cpp:431)
#08: js::InternalCallOrConstruct (M:\src\js\src\vm\Interpreter.cpp:579)
[Child 57664: Main Thread]: I/SelectionAPI 1b24c36f7e0 Selection::ExtendJS(aContainer=span.div.div.div.div.div[contenteditable="true"].div.div.div.div.div.div.div.div.div.div.div.div['t1_jagg4ti'].div.div.div.div.div.div.div.div.div.div.div.div.div.div['AppRouter-main-content'].div.div['SHORTCUT_FOCUSABLE_DIV'].div.div['2x-container'].body.html.#document, aOffset=0)
[Child 57664: Main Thread]: D/SelectionAPI
#01: mozilla::dom::Selection::ExtendJS (M:\src\dom\base\Selection.cpp:2522)
#02: mozilla::dom::Selection_Binding::extend (M:\fx64-dbg\dom\bindings\SelectionBinding.cpp:783)
#03: mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy,mozilla::dom::binding_detail::ThrowExceptions> (M:\src\dom\bindings\BindingUtils.cpp:3320)
#04: CallJSNative (M:\src\js\src\vm\Interpreter.cpp:459)
#05: js::InternalCallOrConstruct (M:\src\js\src\vm\Interpreter.cpp:547)
#06: Interpret (M:\src\js\src\vm\Interpreter.cpp:3362)
#07: js::RunScript (M:\src\js\src\vm\Interpreter.cpp:431)
#08: js::InternalCallOrConstruct (M:\src\js\src\vm\Interpreter.cpp:579)

(The logging feature of Selection API will be available once bug 1818128.

Comment 19

•

2 years ago

Thank you for your hard work!

(In reply to Dennis Schubert [:denschub] from comment #16)

In Chrome, that div has one child node, which contains bbbsomething. However, in Firefox, there's now actually two childNodes, one #text "bbb", and one #text "something". This feels noteworthy, because when I paste a plain-text "something" (in this case copied from my terminal), Firefox, too, only has #text "bbbsomething". Since pasting plain-text works on Reddit, but posting the Bugzilla-copied-"something" does break on Reddit, this extra childNode might be part of the breakage. Before I dig into the contenteditable story myself to figure out why that is and what's going on here, I'll just redirect this question to Masayuki: he knows contenteditable stuff much better than I do. Do you know what is happening here and if this is something we could address?

I think that at pasting text within "text/html" format, it must reach here:
https://searchfox.org/mozilla-central/rev/a2d875255c67e39b28d59a0996e8b54fa2d7564e/editor/libeditor/HTMLEditorDataTransfer.cpp#1137-1140

It just moves topmost children of a node which is in a DocumentFragment created from the dataTransfer data. Yeah, I can fix this to reuse existing text node. Although I can reproduce this bug again, I can try to check it more with fixing the points you found.

Flags: needinfo?(masayuki)

Comment 20

•

2 years ago

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #18)

Oddly, I cannot reproduce this bug today (although the pasting behavior is still odd). Is the intervention is enabled in Reddit?

We did not enable the intervention for Reddit yet, no. We also have no immediate plan to do so, since it's a bit broken either way. I did my tests without the intervention enabled, but I can send you a try push with it if you want to look at that. The behavior is the same for me with or without the intervention, though.

But oddly, I too cannot directly reproduce that with copy-pasting "something". Pasting "something" now works, however the cursor jumps to the beginning of the line, so something is still wonky. However, if I do the same three-line aaa/bbb/ccc test, and instead just copy-paste from something within the editor, like copy'ing ccc and trying to paste it to the end of the bbb line, it still breaks and the entire line disappears.

Reddit did push some JS changes, I think, but in either case, the file where most of the editor content is is still the same, both in terms of hashed filenames, but also in terms of the actual content. So I don't think they did anything to the editor, and I'm not sure why the behavior changed. Since pasting HTML-content that's copied within the editor breaks, and posting plain-text content still works, I think the situation is still the same.

Comment 21

•

2 years ago

I wonder, should I add reddit.com to the block-list pref of the new join-split node direction? Even though I can reproduce the unexpected line break after pasting (I cannot reproduce that the pasting line desappears), but I can reproduce it in both modes. So I think we don't need to use the block-list for reddit.

Comment 22

•

2 years ago

•

Edited

I can still reproduce the originally reported behavior (i.e. the pasting line disappears) very easily, on Win11, nightly 114 (i.e. editor.join_split_direction.compatible_with_the_other_browsers is true). I verified that with a clean profile and trouble shooting mode.
My STR was the same as comment 0.

However, here is an additional note - if I had copied texts from other websites then pasted them over on Reddit first, then when I tried to reproduce comment 0, I didn't seem to see the original issue (i.e. the pasting line disappears) any more. The general pasting behavior was still quite broken anyway.

With editor.join_split_direction.compatible_with_the_other_browsers being false, as noted in several previous comments, the pasting line didn't disappear; however, generally speaking copy/paste still behaves wonky.

I'd like to vote for adding Reddit into the block-list against the new join-split node direction. It's been annoying and confusing on Reddit editing/copy-paste enough and I'd like to avoid more weirdness and changes on it. What do you think?

Updated

•

2 years ago

Flags: needinfo?(masayuki)

Flags: needinfo?(dschubert)

Comment 23

•

2 years ago

Ideally, from a WebCompat perspective, I'd limit the amount of "special cases". If Reddit ever works on their editor to make it more compatible, it would be best if they see the default behavior of Firefox, and not a behavior where parts of the browser behave differently. This is especially relevant if Reddit has development/staging systems on a different domain, which wouldn't see the same blocklist entry, and that could cause even more confusion.

So I don't think I'd add Reddit to the blocklist - unless we have a specific reason for doing so. Currently, it's broken with the pref, and it's broken without that pref. So it's not fixing anything, but it's also not making anything worse as far as I an tell. So I'd leave it at that.

Flags: needinfo?(dschubert)

Comment 24

•

2 years ago

It's hard to say. From our users point of view, some users will see the breakage and they may believe that it's a Firefox's regression. It's bad for both UX and marketing.

However, in this case, Reddit's editor does not work well anyway not only by this issue, and once we use the blocklist, they may never fix it. And currently, we don't have any issues in the wild with the new behavior. Therefore, we have a chance to delete the legacy mode soon if we ignore this issue. Once we use the blocklist, we may never have a chance to remove it. I'm afraid this bad scenario. So, as a developer of Gecko, I recommend not to use the blocklist for this case especially under current situation. On the other hand, it's not user-friendly decision. Therefore, it may be a bad idea from the business point of view.

Flags: needinfo?(masayuki)

Comment 25

•

2 years ago

•

Edited

Thank you Dennis and Masayuki. You made fair points. Then, are we able to get to a summary of what we can do for this issue?

My best understanding of the discussion and diagnosis here was there is no reasonable or small tweak we can do on our side, so we'd leave as it for now. Is it even feasible we have a long-term-ish plan or we will just WONTFIX this? textInput showed up in the diagnosis comments, I was originally hoping that implementing textInputmay help a bit though it doesn't really look like the case ...

Comment 26

•

2 years ago

(In reply to Hsin-Yi Tsai (she/her) [:hsinyi] from comment #25)

My best understanding of the discussion and diagnosis here was there is no reasonable or small tweak we can do on our side,

Yes, I think so.

so we'd leave as it for now.

+1 for this.

Is it even feasible we have a long-term-ish plan or we will just WONTFIX this? textInput showed up in the diagnosis comments, I was originally hoping that implementing textInputmay help a bit though it doesn't really look like the case ...

Yeah, it seems that Reddit's editor has a lot of issues. I guess that one fix would show another issue continuously if you check the fix. So I think that we can do nothing here.

Comment 27

•

2 years ago

•

Edited

I've just taken another look at this, and have noticed that when I copy some text from a Reddit comment box, and then paste it into a plain contenteditable, what's actually being pasted is an empty <div> with a data-reddit-tsjson JSON attribute like {"entityMap":{},"blocks":[{"key":"de7ss","text":"asdf","type":"unstyled","depth":0,"inlineStyleRanges":[],"entityRanges":[],"data":{}}]}. The same happens in Firefox and Chrome, although I do see an interop difference where Chrome pastes it after the current line, regardless of cursor position in the line, while Firefox inserts it at the cursor.

And the modules I'm seeing which mention inlineStyleRanges are indeed draftJS ones such as /node_modules/draft-js/lib/convertFromDraftStateToRaw.js. With luck this clue can help progress the diagnosis here.

Comment 28

•

2 years ago

•

Edited

Ugh. After hours of playing around with this, I finally noticed this CSS in Chrome while comparing behavior:

.public-DraftEditor-content[contenteditable=true] {
    -webkit-user-modify: read-write-plaintext-only;
}

Disabling that makes the editor paste nothing, just like the other contenteditables I played with in comment #27.

So that explains why I couldn't find out what was going on in the JS. They might not be doing anything at all in JS for paste events. In fact this code is in Draft.js, not just Reddit's version: https://github.com/facebookarchive/draft-js/search?q=read-write-plaintext-only

There are likely still other interop issues here, but it feels rather pointless to proceed until we figure out what to do about this, since we don't support read-write-plaintext-only in -moz-user-modify. There is also the plaintext-only value for the contenteditable attribute which we don't support (which I noticed after reading https://chromestatus.com/feature/5743696054059008 )

:masayuki, what do you think?

Flags: needinfo?(masayuki)

Comment 29

•

2 years ago

Actually, it turns out that we can likely make a webcompat intervention for this which emulates the plaintext-only feature. We just have to cancel the paste event, and instead tell Firefox to insert the plain text instead:

const EditorCSS = ".public-DraftEditor-content[contenteditable=true]";

document.documentElement.addEventListener("beforeinput", e => {
  if (e.inputType != "insertFromPaste" || !e.target.closest(EditorCSS)) {
    return;
  }
  const items = e?.dataTransfer.items;
  for (let item of items) {
    if (item.type === "text/plain") {
       e.preventDefault();
       item.getAsString(text => {
          document.execCommand("insertHTML", false, text.replace("\r", "<br>"));
       });
       break;
    }
  }
}, true);

I'm not sure if there are improvements which could be made to that, but it seems to work fine in my quick testing (as Chrome seems to).

It also seems to fit in well with our other draftJS fixes as well, as I'm not seeing any obvious issues with enabling both on Reddit.

Ksenia Berezina [:ksenia] on leave until 09/15

Comment 30

•

2 years ago

Well, we still don't have any plans to prototype nor ship plain-text-only mode of contenteditable because of incompatible between Blink and WebKit. Therefore, sounds reasonable and nice to implement the hack only for specific DraftJS instances.

Flags: needinfo?(masayuki)

Updated

•

2 years ago

Depends on: 1829755

Comment 31

•

2 years ago

For anyone wondering, we just landed this intervention (or rather a tweaked version of it) in bug 1805411, so hopefully the editor will work better in the next nightly builds.

Comment 32

•

2 years ago

Good news so far; one user on Reddit has confirmed that the webcompat intervention seems to be working properly for them on the latest nightly build. I've requested feedback from other users just in case we missed anything, but the fix is due to ship with Firefox 114.