1841656 - Firefox Translations doesn't deal with Shadow DOM

Status: RESOLVED FIXED

(Firefox :: Translations, defect, P3)

Description

[Mozinet]

Attachment: Partially translated page in French

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:109.0) Gecko/20100101 Firefox/117.0

Steps to reproduce:

Open https://ko-fi.com/post/If-You-Suffer-from-Urgent-Normal-Syndrome-Ask-for-K3K6MN65U in Firefox Nightly.

Translate the page with the built-in Firefox Translations feature, in French, for example.

Actual results:

The title of the article and the page menus have been translated, however, the article text remains in its original English language.

Expected results:

All the text of the page needs to be translated.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

This is a more general problem. The article contents are inside a shadow root. Greg, Translations don't seem to handle shadow dom at all which seems like it could affect a bunch of pages.

Comment 2

[Greg Tatum [:gregtatum]]

Seems the TreeWalker class doesn't handle the shadow DOM. I found some discussion here: https://github.com/whatwg/dom/issues/665

I guess we'll need to find a way to recurse into it. The extension also can't handle the shadow DOM.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

We expose .openOrClosedShadowRoot to extensions and chrome code.

Comment 4

[Greg Tatum [:gregtatum]]

So it looks like we'll need to call addRootElement for shadow DOMs that we encounter while walking the nodes.

Comment 5

[Marco Castelluccio [:marco]]

The problem is also reproducible on https://www.suewag.de/privatkunden.

Comment 6

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

The severity field is not set for this bug.
:nordzilla, could you have a look please?

For more information, please visit BugBot documentation.

Comment 7

[Sean Feng [:sefeng]]

Attachment: Bug 1841656 - Make Firefox Translation iterates nodes inside ShadowDOM r=gregtatum

Comment 8

[Greg Tatum [:gregtatum]]

The test page isn't loading for me on Nightly. I can't confirm the fix, and I'm having trouble finding examples to try.

Comment 9

[Sean Feng [:sefeng]]

Attachment: simple-testcase.html

Here's a simple test case for this.

I just updated the patch (there's was a bug in my original approach), and have tested it with both sites.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c12f569ae50b
Make Firefox Translation iterates nodes inside ShadowDOM r=gregtatum

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/c12f569ae50b

Comment 12

[Marco Castelluccio [:marco]]

Sean, what do you think of uplifting the fix to 119 or even 118?

Comment 13

[Sean Feng [:sefeng]]

Macro found out some content on https://www.suewag.de/privatkunden were still not being translated.

It turned out there were two issues. One related to slotted content and the other related to ShadowDOM mutations.

I plan to land a patch in Bug 1855307 to address those. We have an existing bugs open for ShadowDOM mutations as Bug 1842820, unfortunately I tested it with my current approach for ShadowDOM mutations and it didn't fix it. So I'll leave Bug 1842820 as is.

Leave my NI for uplifting this patch along with Bug 1855307.

Comment 15

[Sean Feng [:sefeng]]

Comment on attachment 9354420 [details]
Bug 1841656 - Make Firefox Translation iterates nodes inside ShadowDOM r=gregtatum

Beta/Release Uplift Approval Request

• User impact if declined: Content in shadow dom won't be translated by Firefox Translation
• 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: Bug 1855307
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is low risky because the change is trivial and is covered by automated test.
• String changes made/needed:
• Is Android affected?: No

Comment 16

[Sean Feng [:sefeng]]

Comment on attachment 9354420 [details]
Bug 1841656 - Make Firefox Translation iterates nodes inside ShadowDOM r=gregtatum

Beta/Release Uplift Approval Request

• User impact if declined: Content in shadow dom will not be translated
• 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: Bug 1855307
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is low risky because the change is trivial and is covered by automated test
• String changes made/needed:
• Is Android affected?: No

Comment 17

[Dianna Smith [:diannaS] - on Leave, back on August 9th]

Comment on attachment 9354420 [details]
Bug 1841656 - Make Firefox Translation iterates nodes inside ShadowDOM r=gregtatum

Approved for 119.0b5

Comment 18

[Pulsebot]

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

Comment 19

[Pascal Chevrel:pascalc]

Comment on attachment 9354420 [details]
Bug 1841656 - Make Firefox Translation iterates nodes inside ShadowDOM r=gregtatum

Approved for our next 118 dot release, thanks.

Comment 20

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/b59b2ac81d43

Comment 21

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-release/rev/b59b2ac81d43