1921146 - Caret does not appear in the input field contenteditable on https://chatgpt.com/ and also on `AI Chatbot` sidebar(Settings > Firefox Labs > AI Chatbot > ChatGPT)

Status: VERIFIED FIXED

(Core :: DOM: Selection, defect)

Description

[Alice0775 White]

Steps to reproduce:

1. Open https://chatgpt.com/ or Open AI Chatbot sidebar(Settings > Firefox Labs > AI Chatbot > ChatGPT)
2. Clock on Message ChatGPT input field

Actual results:
No caret is displayed.

Expected results:
Caret should be displayed.
Chrome works as expected.

Comment 1

[Alice0775 White]

Attachment: screenshot

Comment 2

[Jan Jaeschke [:jjaschke]]

Can repro on Nightly, but not on Release (both macOS Sonoma). It does not look like a recent regression, ./mach mozregression -g 90 can still reproduce the issue.

Comment 3

[mike.clark.8192]

I also experienced this issue. A few days ago I made a workaround/fix userscript for myself. If you'd like to try it, it's on the website GreasyFork under the name "Firefox/ChatGPT: Fix Disabled Chat Box".

Based on the workaround approach I had to take, I think it might be a bug or race condition in the ChatGPT website's browser detection logic. I don't know what the root cause of the browser detection failure / race is, but the workaround I came up with does what I think website's browser detection "should" be doing, but later in the page load.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

Since nightly and release are affected, beta will likely be affected too.
For more information, please visit BugBot documentation.

Comment 5

[Haik Aftandilian [:haik]]

Affected platforms include macOS and Windows.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

The patch will be too risky for a dot release.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Hmm, fixing this causes hitting the failure handled in bug 1921701...

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

And it seems that current accessible caret handling depends on this bug. Once we start to use parent frame to put caret if it has only invisible children, accessible caret may appear at unexpected position.

Comment 9

[mike.clark.8192]

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

Hmm, fixing this causes hitting the failure handled in bug 1921701...

What is the fix you are thinking of for this issue and how does it break the fix for bug1921701? Sorry if it is already visible, I am new to Bugzilla. I read bug1921701, which gives me some context, but I maybe not enough.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Ah, okay, I got it. I'll post a patch soon.

mike.clark.8192: When I touched the getter method logic which returns a layout object to consider the caret position, it also changed the behavior of caret move in some cases. However, I found a way to avoid that now.

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1921146 - Make `SelectionMovementUtils::GetFrameForNodeOffset` use last frame when the loop ended with invisible content r=emilio!,#dom-core

The editor in ChatGPT has only invisible <br> (display: none) and text
content as ::after. SelectionMovementUtils::GetFrameForNodeOffset assumes
that at least one child has a primary frame. Therefore, it may return nullptr
and nsCaret fails to paint the caret due to no frame. So, it should use a
parent frame in such case.

However, doing it may cause an assertion failure [1] when
IMEContentObserver::UpdateSelectionCache() is called while an editor gets
focus and the editor does not have visible children. Therefore, this patch
also tries to make it run asynchronously in content process. However, it was
changed to sync-handling in bug 1385666. However, the IME related code around
the process boundary and TSFTextStore are improved a lot after fixing the bug.
Therefore, now, it could be no problem with handling asynchronously. However,
unfortunately, the web site was updated to not use the WYSIWYG editing most.
Therefore, I cannot check the behavior and I'd like to test it for a while
in early beta builds and earlier.

1. https://searchfox.org/mozilla-central/rev/7e0ae4372c52b8183d1178132dd6493edb576738/layout/generic/nsIFrame.cpp#2039

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/02bd95b16de9
Make `SelectionMovementUtils::GetFrameForNodeOffset` use last frame when the loop ended with invisible content r=emilio

Comment 13

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/48484 for changes under testing/web-platform/tests

Comment 14

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/02bd95b16de9

Comment 15

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I'd be happy if somebody checked whether it's fixed in nightly builds with/without login.

Comment 17

[Jan Jaeschke [:jjaschke]]

I can't reproduce the issue in yesterday's Nightly.

Comment 18

[Alice0775 White]

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

I'd be happy if somebody checked whether it's fixed in nightly builds with/without login.

The issue is no longer reproduced with/without logged-in on Nightly133.0a1(20241006215200) Windows11.

Comment 19

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:masayuki, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox132 to wontfix.

For more information, please visit BugBot documentation.

Comment 20

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Thank you! Then, let's request uplift to beta!

Comment 21

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 9428963 [details]
Bug 1921146 - Make SelectionMovementUtils::GetFrameForNodeOffset use last frame when the loop ended with invisible content r=emilio!,#dom-core

Beta/Release Uplift Approval Request

• User impact if declined: This happens in ChatGPT which must be one of major web services. And this bug makes it harder to typing text. So, this is really annoying for users.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This changes only the caret painting path to use parent frame when there are some children in caret container but all of them are invisible. This situation itself is not major case.
• String changes made/needed: none
• Is Android affected?: Yes

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9428963 [details]
Bug 1921146 - Make SelectionMovementUtils::GetFrameForNodeOffset use last frame when the loop ended with invisible content r=emilio!,#dom-core

Approved for 132.0b5.

Comment 23

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/3bec591930f3