Closed Bug 1927838 Opened 1 year ago Closed 10 months ago

Monaco Editor inside of a webcomponent does not work any more in FF > 130

Categories

(Web Compatibility :: Site Reports, defect, P1)

Firefox 131

Tracking

(Webcompat Priority:P1, Webcompat Score:9, firefox-esr115 unaffected, firefox-esr128 unaffected, firefox132 wontfix, firefox133 disabled, firefox134+ disabled)

RESOLVED WORKSFORME
Webcompat Priority P1
Webcompat Score 9
Tracking Status
firefox-esr115 --- unaffected
firefox-esr128 --- unaffected
firefox132 --- wontfix
firefox133 --- disabled
firefox134 + disabled

People

(Reporter: jochen.kuehner, Unassigned)

References

(Blocks 1 open bug, Regression, )

Details

(5 keywords, Whiteboard: [webcompat:sightline])

User Story

platform:windows,mac,linux
impact:workflow-broken
configuration:general
affects:all
branch:release
user-impact-score:800

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/129.0.0.0 Safari/537.36

Steps to reproduce:

You could not Edit Text in Monaco Editor if it is hosted in a Webcomponent.
1.) Goto https://microsoft.github.io/monaco-editor/playground.html?source=v0.52.0#example-creating-the-editor-hello-world
2.) Switch in DropDown Example to "Web Component" (2nd Entry)
3.) Try to set the cursor inside of the Text in the right Editor. It only works if you click at the end of the text.

Actual results:

I tried to set the cursor in a monaco editor (inside of a webcoponent) to a position inbetween the text. This works in Firefox 130 but does not in 131 and greater.

Expected results:

You should be able to position the cursor

The Bugbug bot thinks this bug should belong to the 'Core::DOM: Editor' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Component: Untriaged → DOM: Editor
Product: Firefox → Core

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]: Web based code editor is broken.

I can reproduce the issue on Nightly134.0a1 Windows11.

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=158f4c78fb00946a58dc6d6f0bcee7ded99ef210&tochange=f022eaedf70e9091aecee2cd90d2eba8b127d392

Status: UNCONFIRMED → NEW
Ever confirmed: true
Regressed by: 1889503

[Tracking Requested - why for this release]:

:gregp, since you are the author of the regressor, bug 1889503, could you take a look?

For more information, please visit BugBot documentation.

Flags: needinfo?(gregp)

At first I was surprised that this isn't broken in Chromium. In Chromium, caretRangeFromPoint does not pierce shadow dom at all. And Chromium has also shipped caretPositionFromPoint api recently with the same behavior1.

It turns out, Monaco has a special shadowCaretRangeFromPoint2 path to deal with this, but they don't apply it to caretPositionFromPoint3.

It seems selection isn't broken in Chromium because Monaco checks for caretRangeFromPoint before caretPositionFromPoint4. Since Chromium ships both APIs, the happy shadowCaretRangeFromPoint path will be taken and everything will just work.

I think the easiest way to fix this is to ship caretRangeFromPoint API. I don't think it will ever be possible for Chromium/Safari to unship it because usage is very high...
https://chromestatus.com/metrics/feature/timeline/popularity/387

Flags: needinfo?(gregp)
Regressed by: 1914596
No longer regressed by: 1889503
Depends on: 1550635

Is this something we could create an intervention for given comment 5?

Flags: needinfo?(dschubert)

(In reply to Gregory Pappas [:gregp] from comment #5)

I think the easiest way to fix this is to ship caretRangeFromPoint API.

How easy or difficult would that be? Who would be the right person to do it? (I see you've recently modified the other API.)

Flags: needinfo?(gregp)
Flags: needinfo?(twisniewski)

(In reply to Henri Sivonen (:hsivonen) from comment #7)

How easy or difficult would that be? Who would be the right person to do it? (I see you've recently modified the other API.)

It should be pretty easy, I think. I posted a patch which does so in bug 1550635 which does fix Monaco in a local build.

One concern is that shipping caretRangeFromPoint could introduce different web compat issues from sites that use it for UA detection...

Flags: needinfo?(gregp)

Thanks. I think the way forward is pursuing landing bug 1550635 (with the current review comments addressed).

My only concern with that is how upliftable it'll be. I'm wondering if we can craft an intervention in the mean time.

The bug is marked as tracked for firefox134 (nightly). We have limited time to fix this, the soft freeze is in 14 days. However, the bug still isn't assigned.

:hsinyi, could you please find an assignee for this tracked bug? Given that it is a regression and we know the cause, we could also simply backout the regressor. If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Flags: needinfo?(htsai)

(In reply to BugBot [:suhaib / :marco/ :calixte] from comment #11)

The bug is marked as tracked for firefox134 (nightly). We have limited time to fix this, the soft freeze is in 14 days. However, the bug still isn't assigned.

:hsinyi, could you please find an assignee for this tracked bug? Given that it is a regression and we know the cause, we could also simply backout the regressor. If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

The platform fix is being worked on in bug 1550635.

Severity: -- → S3
Flags: needinfo?(htsai)
Priority: -- → P2

This is a reminder regarding comment #11!

The bug is marked as tracked for firefox134 (nightly). We have limited time to fix this, the soft freeze is in 8 days. However, the bug still isn't assigned.

Do we have a list of sites which are broken by this beyond the demo/playground page? I can try to write a webcompat intervention to polyfill specific sites, at least, until we try shipping caretRangeFromPoint. Simon, I wonder if you have any magic CRUX queries?

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

This is a reminder regarding comment #11!

The bug is marked as tracked for firefox134 (nightly). We have limited time to fix this, the soft freeze is in 2 days. However, the bug still isn't assigned.

I am actively working on bug 1550635. I've done a lot more testing and figured out that Webkit and Blink both have some interesting behavior that is hard (but not impossible) to emulate in Gecko. I believe getting close to bug-for-bug compatibility is necessary because sites tend to use caretRangeFromPoint before caretPositionFromPoint. So, shipping caretRangeFromPoint runs the risk of introducing even worse, even more widespread breakage...

Because of this, I now believe that we should:

  • Back out bug 1914596
  • Land caretRangeFromPoint in Nightly
  • Wait a while to make sure nothing important breaks
  • Release bug 1914596 and caretRangeFromPoint in the same release, to avoid breakage like bug 1927838

(In reply to Gregory Pappas [:gregp] from comment #16)

I am actively working on bug 1550635. I've done a lot more testing and figured out that Webkit and Blink both have some interesting behavior that is hard (but not impossible) to emulate in Gecko. I believe getting close to bug-for-bug compatibility is necessary because sites tend to use caretRangeFromPoint before caretPositionFromPoint. So, shipping caretRangeFromPoint runs the risk of introducing even worse, even more widespread breakage...

Because of this, I now believe that we should:

  • Back out bug 1914596
  • Land caretRangeFromPoint in Nightly
  • Wait a while to make sure nothing important breaks
  • Release bug 1914596 and caretRangeFromPoint in the same release, to avoid breakage like bug 1927838

Thank you :gregp for the explanation and details.
Olli, you reviewed the patches for these features. What do you think about the recommendation above?

And I'm guessing that Back out bug 1914596 literally means change the preferences back to Nightly_only and uplift it up to the earliest version where feasible and reasonable.

Also flagging Donal to get right attention.

Flags: needinfo?(smaug)
Flags: needinfo?(dmeehan)

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

(In reply to Gregory Pappas [:gregp] from comment #16)

I am actively working on bug 1550635. I've done a lot more testing and figured out that Webkit and Blink both have some interesting behavior that is hard (but not impossible) to emulate in Gecko. I believe getting close to bug-for-bug compatibility is necessary because sites tend to use caretRangeFromPoint before caretPositionFromPoint. So, shipping caretRangeFromPoint runs the risk of introducing even worse, even more widespread breakage...

Because of this, I now believe that we should:

  • Back out bug 1914596
  • Land caretRangeFromPoint in Nightly
  • Wait a while to make sure nothing important breaks
  • Release bug 1914596 and caretRangeFromPoint in the same release, to avoid breakage like bug 1927838

Thank you :gregp for the explanation and details.
Olli, you reviewed the patches for these features. What do you think about the recommendation above?

And I'm guessing that Back out bug 1914596 literally means change the preferences back to Nightly_only and uplift it up to the earliest version where feasible and reasonable.

Also flagging Donal to get right attention.

:hsinyi I will likely have an Fx133 RC2 that will build tomorrow. I can confirm that Bug 1914596 backs out cleanly from release.
Standing by for a response from :smaug to Comment 17 before looping back on the discussion.

Flags: needinfo?(dmeehan)

the plan sounds reasonable.

Flags: needinfo?(smaug)

Setting Fx134 to disabled since the regressor was backed out
https://bugzilla.mozilla.org/show_bug.cgi?id=1914596#c5

Release backout will follow

Setting Fx133 to disabled since the regressor was backed out of release
https://bugzilla.mozilla.org/show_bug.cgi?id=1914596#c6

Flags: needinfo?(dschubert)

(In reply to Thomas Wisniewski [:twisniewski] from comment #14)

Do we have a list of sites which are broken by this beyond the demo/playground page? I can try to write a webcompat intervention to polyfill specific sites, at least, until we try shipping caretRangeFromPoint. Simon, I wonder if you have any magic CRUX queries?

I queried for pages that use Monaco Editor and customElements.define() (though the usage of those might be separate), ordered by rank:

https://docs.google.com/spreadsheets/d/18LL5mW5beunYlvoLIvfAweZZ0KdcZpbFQXPW6YOnfY0/edit?usp=sharing

Flags: needinfo?(zcorpan)

The bug should have been fixed in release since Fx 133, since the regressor bug 1914596 was backed out.

Component: DOM: Editor → Site Reports
Product: Core → Web Compatibility
Webcompat Priority: --- → P3
Webcompat Score: --- → 2

Setting the URL for this bug to https://www.tradingview.com/ as it's the highest-ranking site that might be using this editor and might be broken according to Comment 22. Also, I'm adding webcompat:blocked - this technically isn't broken right now, but we will be landing bug 1914596 at some point, so we need to keep this bug open as a reminder not to break the world if we do land that.

Severity: S3 → S2
User Story: (updated)
Webcompat Priority: P3 → P1
Webcompat Score: 2 → 8
Priority: P2 → P1
Whiteboard: [webcompat:sightline]
Webcompat Score: 8 → 9

Since this is currently fixed, should we just add notes to bug 1914596 about not breaking this usecase when they reland, and close this out?

Flags: needinfo?(dschubert)

I'm fine with closing this for now - seems like the dependencies are pretty clear based on bug 1914596 comment 14.

Status: NEW → RESOLVED
Closed: 10 months ago
Flags: needinfo?(dschubert)
Resolution: --- → WORKSFORME
No longer depends on: 1550635, 1914596

Not really news, just noting that things haven't magically changed out from under us here [as they sometimes do when sites ship redesigns/upgrades/etc]:

I just retested & confirmed that this is still an issue (in Nightly, where dom.shadowdom.new_caretPositionFromPoint_behavior.enabled is true).

I tested using STR in comment 0, and I confirmed that I'm unable to click around in the "preview" area (except at the very end) when the pref is true - but setting it to false restores the good behavior (and that's the value we're currently shipping on release). See attached screencast (where I mouse across the Preview section and click at various points to try to move the caret, but fail to do so when the pref is true).

I'm using Nightly 147.0a1 (2025-12-02).

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: