site-kit-wp icon indicating copy to clipboard operation
site-kit-wp copied to clipboard

Fix current instabilities with Google Translate

Open aaemnnosttv opened this issue 2 years ago • 11 comments

Feature Description

As a product used by millions globally, it's no surprise that some users use Google Translate on Site Kit interfaces. As identified in the past, this can result in fatal application errors when React attempts to rerender and gets "confused" due to DOM manipulations made by Google Translate.

In our error reporting (for users who opt-in – thank you 😄), we can see a number of similar errors that are indicative of this kind of this problem:

e.g.

Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node.

Related issues

  • #3636
  • #2280

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • SK screens should be audited and updated for compatibility with Google Translate, specifically for cases where the dashboard crashes due to an update after translation (i.e. Failed to execute 'removeChild' on 'Node')

Implementation Brief

  • In assets/js/components/dashboard-sharing/DashboardSharingDialog/index.js:
    • Define a new variable that will hold the current dynamic text, and assign it based on the existing conditions. By default set it to empty string.
      • Render text from new variable to the referenced code, instead of dynamically
    • Define another variable for the dynamic text in paragraph as well, and do the same as previously for h2 text
  • Other components work, but in just case do an overall check if there is similar error when GTranslate extension is active

Test Coverage

  • No updates needed

QA Brief

Changelog entry

aaemnnosttv avatar Feb 03 '23 22:02 aaemnnosttv

With Site Kit 1.110.0 I encountered errors documented in this task.

  • Dashboard Sharing: issues when closing the popup if translations were active on the popup (screenshot)
  • Hovering over the SK admin toolbar once a page is already translated: formatting and console error - screenshot. Possibly not unexpected, with the content here loaded after the initial page load. The issue doesn’t occur instantly. To reproduce this, use the GTranslate plugin to load a front end URL, change the language, then change the language once more before viewing the SK admin toolbar.
  • Hovering over the “Create a conversion” button (Chrome extension only): Sporadic console errors and formatting on the tooltip. (screenshot). I was able to recreate this 2 or 3 times but not consistently. On one occasion I switched the reporting period after translating the page, then waited until the tooltip occurred. I couldn’t repeat this from further attempts. Note that I did encounter this elsewhere also.

Happy to conduct further testing if required.

jamesozzie avatar Oct 09 '23 15:10 jamesozzie

@bethanylang this is probably best continued outside of GH since this is essentially a task to inform new issues. Let's close this and pick up in a new task.

aaemnnosttv avatar Dec 19 '23 21:12 aaemnnosttv

Thanks @aaemnnosttv! I checked the related Asana task and it looks like back in October you had asked for the issues that James identified to be added here in Github and we closed the Asana task. What's the next step here?

mxbclang avatar Dec 20 '23 12:12 mxbclang

Thanks @bethanylang – we want to fix the issues with Google Translate that we're aware of, so we can redefine this issue or close it an open a new one to address these.

To be clear, we're looking to fix issues where the dashboard crashes (e.g. https://imgur.com/z3Mli0u) – we're not sure that the other examples shared (console errors) were from SK or not, but those aren't the issue we're looking to fix.

I'll update the issue so this can move forward.

aaemnnosttv avatar Dec 22 '23 04:12 aaemnnosttv

Couldn't reproduce this error. Tried translating on main dashboard when all data is loaded, and before it is loaded. Tried the same on single page view as well

zutigrm avatar Dec 22 '23 12:12 zutigrm

@zutigrm I added some details from testing this today. You'll find details on this in this Asana task.

In my case I was only able to recreate issues as I was previously in one case, when using the Google Translate chrome extension and then closing the Dashboard Sharing modal. Details in that task.

jamesozzie avatar Dec 29 '23 16:12 jamesozzie

@zutigrm Reassigning to you to review James' update, please. If you don't feel like you're the best person to do the IB, feel free to unassign yourself. :)

mxbclang avatar Jan 08 '24 14:01 mxbclang

@bethanylang Thanks, it seems I missed the previous comment

zutigrm avatar Jan 08 '24 14:01 zutigrm

@zutigrm, I am not sure that your IB changes anything. Previously, in the past, we needed to wrap a text into a span to make it work correctly when Google Translate was used (see #3636). I think we probably need to do the same in this ticket as well, right?

cc @aaemnnosttv or @tofumatt in case they want to add something.

eugene-manuilov avatar Jan 09 '24 17:01 eugene-manuilov

@eugene-manuilov The problem was when two text content are rendered conditionally, I did I quick test of the proposed implementation, it fixed the issue, so either approach should be valid.

But let's see if @aaemnnosttv or @tofumatt have something to add

zutigrm avatar Jan 10 '24 10:01 zutigrm

Usually we've added the span instead, I'd rather us stick with a consistent implementation, which would be using a span, but as long as it fixes it I don't have anything to add here.

tofumatt avatar May 15 '24 14:05 tofumatt

+1 to what @tofumatt said. Let's revisit this with a fresh check to see if there are any other instances which need fixing here as it's very likely that new issues have come up.

Beyond that, this seems like something we could avoid going forward with an ESLint rule to detect when a non-GTranslate safe condition is used 🤔 WDYT @tofumatt ?

aaemnnosttv avatar Jan 24 '25 19:01 aaemnnosttv