FIX: improves text selection of posts

jjaffeux · jjaffeux · commit a929410d44ad · 2025-06-11T18:24:54.000+02:00
This commit is applying different techniques to make selecting text of a post less error prone:
- disables text selection of all the page BUT the current post
- ensures the selection menu is not interfering with text selection
- ensures the d-header is not interfering with text selection

The situation was very bad on android but it should also improve other situations.
diff --git a/app/assets/javascripts/discourse/app/components/post-text-selection.gjs b/app/assets/javascripts/discourse/app/components/post-text-selection.gjs
@@ -5,11 +5,13 @@ import { cancel, debounce } from "@ember/runloop";
 import { service } from "@ember/service";
 import { modifier } from "ember-modifier";
 import PostTextSelectionToolbar from "discourse/components/post-text-selection-toolbar";
+import bodyClass from "discourse/helpers/body-class";
 import discourseDebounce from "discourse/lib/debounce";
 import { bind } from "discourse/lib/decorators";
 import { INPUT_DELAY } from "discourse/lib/environment";
 import escapeRegExp from "discourse/lib/escape-regexp";
 import isElementInViewport from "discourse/lib/is-element-in-viewport";
+import discourseLater from "discourse/lib/later";
 import toMarkdown from "discourse/lib/to-markdown";
 import { applyValueTransformer } from "discourse/lib/transformer";
 import {
@@ -49,7 +51,8 @@ export default class PostTextSelection extends Component {
   @service siteSettings;
   @service menu;
 
-  @tracked isSelecting = false;
+  @tracked hasPostTextSelection = false;
+  @tracked selectTextMode = false;
   @tracked preventClose = applyValueTransformer(
     "post-text-selection-prevent-close",
     false
@@ -64,14 +67,24 @@ export default class PostTextSelection extends Component {
   });
 
   documentListeners = modifier(() => {
-    document.addEventListener("mousedown", this.mousedown, { passive: true });
-    document.addEventListener("mouseup", this.mouseup, { passive: true });
     document.addEventListener("selectionchange", this.onSelectionChanged);
+    document.addEventListener("pointerup", this.pointerup, { passive: true });
+    // fires when user finishes adjusting text selection on android
+    document.addEventListener("contextmenu", this.contextmenu, {
+      passive: true,
+    });
+    document.addEventListener("touchend", this.touchend, {
+      passive: true,
+    });
 
     return () => {
-      document.removeEventListener("mousedown", this.mousedown);
-      document.removeEventListener("mouseup", this.mouseup);
       document.removeEventListener("selectionchange", this.onSelectionChanged);
+      document.removeEventListener("pointerup", this.pointerup);
+      document.removeEventListener("contextmenu", this.contextmenu);
+      document.removeEventListener("touchend", this.touchend);
+
+      this._cleanUserSelectState();
+      this.hasPostTextSelection = false;
     };
   });
 
@@ -93,6 +106,7 @@ export default class PostTextSelection extends Component {
     super.willDestroy(...arguments);
 
     cancel(this.debouncedSelectionChanged);
+    cancel(this.touchEndLaterHandler);
     this.menuInstance?.close();
   }
 
@@ -105,11 +119,7 @@ export default class PostTextSelection extends Component {
     await this.menuInstance?.close();
   }
 
-  async selectionChanged(options = {}) {
-    if (this.isSelecting) {
-      return;
-    }
-
+  async selectionChanged(options = {}, cooked, postId) {
     const _selectedText = selectedText();
 
     const selection = window.getSelection();
@@ -134,29 +144,6 @@ export default class PostTextSelection extends Component {
 
     this.prevSelectedText = _selectedText;
 
-    // ensure we selected content inside 1 post *only*
-    let postId;
-    for (let r = 0; r < selection.rangeCount; r++) {
-      const range = selection.getRangeAt(r);
-      const selectionStart = getElement(range.startContainer);
-      const ancestor = getElement(range.commonAncestorContainer);
-
-      if (!selectionStart.closest(".cooked")) {
-        return await this.hideToolbar();
-      }
-
-      postId ||= ancestor.closest(".boxed, .reply")?.dataset?.postId;
-
-      if (!ancestor.closest(".contents") || !postId) {
-        return await this.hideToolbar();
-      }
-    }
-
-    const _selectedElement = getElement(selectedNode());
-    const cooked =
-      _selectedElement.querySelector(".cooked") ||
-      _selectedElement.closest(".cooked");
-
     // computing markdown takes a lot of time on long posts
     // this code attempts to compute it only when we can't fast track
     let opts = {
@@ -166,6 +153,7 @@ export default class PostTextSelection extends Component {
           : _selectedText === toMarkdown(cooked.innerHTML),
     };
 
+    const _selectedElement = getElement(selectedNode());
     for (
       let element = _selectedElement;
       element && element.tagName !== "ARTICLE";
@@ -185,7 +173,6 @@ export default class PostTextSelection extends Component {
     let supportsFastEdit = this.canEditPost;
 
     const start = getElement(selection.getRangeAt(0).startContainer);
-
     if (!start || start.closest(CSS_TO_DISABLE_FAST_EDIT)) {
       supportsFastEdit = false;
     }
@@ -220,7 +207,8 @@ export default class PostTextSelection extends Component {
         //   so we need more space
         // - the end of the selection is not in viewport, in this case our menu will be shown at the top
         //   of the screen, so we need more space to avoid overlapping with the native menu
-        offset = 70;
+        const { isAndroid } = this.capabilities;
+        offset = isAndroid ? 90 : 70;
       }
     }
 
@@ -257,29 +245,59 @@ export default class PostTextSelection extends Component {
   }
 
   @bind
-  onSelectionChanged() {
-    if (this.isSelecting) {
-      return;
+  async onSelectionChanged(options) {
+    const selection = window.getSelection();
+    if (selection.rangeCount) {
+      const range = selection.getRangeAt(0);
+      const parent =
+        range.commonAncestorContainer.nodeType === Node.TEXT_NODE
+          ? range.commonAncestorContainer.parentNode
+          : range.commonAncestorContainer;
+
+      const cooked = parent.closest(".cooked");
+      if (cooked) {
+        const article = cooked.closest(".boxed, .reply");
+        const postId = article.dataset.postId;
+
+        if (!options.force) {
+          this.selectTextMode = true;
+          this.hasPostTextSelection = true;
+        }
+
+        const { isIOS, isWinphone, isAndroid } = this.capabilities;
+        const wait = isIOS || isWinphone || isAndroid ? INPUT_DELAY : 25;
+        this.selectionChangeHandler = discourseDebounce(
+          this,
+          this.selectionChanged,
+          options,
+          cooked,
+          postId,
+          wait
+        );
+      } else {
+        cancel(this.selectionChangeHandler);
+        await this.hideToolbar();
+      }
     }
+  }
 
-    const { isIOS, isWinphone, isAndroid } = this.capabilities;
-    const wait = isIOS || isWinphone || isAndroid ? INPUT_DELAY : 25;
-    this.selectionChangeHandler = discourseDebounce(
-      this,
-      this.selectionChanged,
-      wait
-    );
+  @bind
+  touchend() {
+    // ensures touchend is processed after selectionchange
+    // this is especially needed on iOS
+    this.touchEndLaterHandler = discourseLater(() => {
+      this._cleanUserSelectState();
+    }, 50);
   }
 
   @bind
-  mousedown() {
-    this.isSelecting = true;
+  contextmenu() {
+    this._cleanUserSelectState();
   }
 
   @bind
-  mouseup() {
-    this.isSelecting = false;
-    this.onSelectionChanged();
+  pointerup() {
+    this._cleanUserSelectState();
   }
 
   get post() {
@@ -307,9 +325,10 @@ export default class PostTextSelection extends Component {
   @action
   handleTopicScroll() {
     if (this.site.mobileView) {
+      this._cleanUserSelectState();
       this.debouncedSelectionChanged = debounce(
         this,
-        this.selectionChanged,
+        this.onSelectionChanged,
         { force: true },
         250,
         false
@@ -328,7 +347,51 @@ export default class PostTextSelection extends Component {
     return await this.args.buildQuoteMarkdown();
   }
 
+  _cleanUserSelectState() {
+    document
+      .querySelector(".allow-post-text-selection")
+      ?.classList.remove("allow-post-text-selection");
+
+    this.selectTextMode = false;
+
+    const selection = window.getSelection();
+    if (selection.rangeCount) {
+      const range = selection.getRangeAt(0);
+      const selectionStart = getElement(range.startContainer);
+      const cooked = selectionStart.closest(".cooked");
+      this.hasPostTextSelection = !!cooked;
+    } else {
+      this.hasPostTextSelection = false;
+    }
+  }
+
   <template>
+    {{! styles are inline to ensure browser has to parse them only when necessary}}
+    {{#if this.selectTextMode}}
+      {{bodyClass "-select-post-text-mode"}}
+
+      <style>
+        body.-select-post-text-mode {
+          [data-identifier="post-text-selection-toolbar"] {
+            display: none;
+          }
+        }
+      </style>
+    {{/if}}
+
+    {{#if this.hasPostTextSelection}}
+      {{bodyClass "-has-post-text-selection"}}
+
+      <style>
+        body.-has-post-text-selection {
+          .d-header-wrap * {
+            user-select: none;
+            -webkit-user-select: none;
+          }
+        }
+      </style>
+    {{/if}}
+
     <div
       {{this.documentListeners}}
       {{this.appEventsListeners}}
diff --git a/app/assets/javascripts/discourse/app/lib/utilities.js b/app/assets/javascripts/discourse/app/lib/utilities.js
@@ -122,12 +122,6 @@ export function selectedText() {
         ? range.commonAncestorContainer
         : range.commonAncestorContainer.parentElement;
 
-    // ensure we never quote text in the post menu area
-    const postMenuArea = ancestor.querySelector(".post-menu-area");
-    if (postMenuArea) {
-      range.setEndBefore(postMenuArea);
-    }
-
     const oneboxTest = ancestor.closest("aside.onebox[data-onebox-src]");
     const codeBlockTest = ancestor.closest("pre");
     if (codeBlockTest) {
diff --git a/app/assets/stylesheets/common/base/topic-post.scss b/app/assets/stylesheets/common/base/topic-post.scss
@@ -737,10 +737,6 @@ aside.quote {
   opacity: 0.4;
 }
 
-.fk-d-menu[data-identifier="post-text-selection-toolbar"] {
-  @include user-select(none);
-}
-
 .quote-button {
   flex-direction: column;
 

Original file line number	Diff line number	Diff line change
`@@ -737,10 +737,6 @@ aside.quote {`
`737`	`737`	`opacity: 0.4;`
`738`	`738`	`}`
`739`	`739`
`740`		`-.fk-d-menu[data-identifier="post-text-selection-toolbar"] {`
`741`		`- @include user-select(none);`
`742`		`-}`
`743`		`-`
`744`	`740`	`.quote-button {`
`745`	`741`	`flex-direction: column;`
`746`	`742`