preactjs / preact

⚛️ Fast 3kB React alternative with the same modern API. Components & Virtual DOM.
https://preactjs.com
MIT License
36.83k stars 1.95k forks source link

Chrome's automatic page translation breaking Preact rendering #948

Open dharFr opened 6 years ago

dharFr commented 6 years ago

It looks like Preact and Chrome automatic translation feature aren't best friends.

As far as I can tell, it can be reproduced on any Preact based web app, but I made a simple test page just to be sure: http://chrome-translate-preact---not.surge.sh/

1- enable Chrome's page translation

step-1

2- interact with the app

step-2

EDIT : I don't really know where to start to fix this, but please tell me if I can help somehow.

developit commented 6 years ago

Need to take a peek at what Chrome is doing under the hood here. It seems like it's flat-out replacing Text nodes, but that is really odd. It should just be setting their .nodeValue in-place.

tomprats commented 6 years ago

I'm also running into this issue and the only "fix" I've been able to find for it is just disabling the translation with by adding this meta tag to the head: <meta name="google" value="notranslate">.

Any ideas on how to get around this issue while still allowing Google to translate the page?

developit commented 6 years ago

I think we might have to wait for the next version of Preact to fix this, because right now we store too much state in the DOM.

Nopzen commented 6 years ago

@developit I'am experience the same issue aswell, not to push for it even further but just to let you know of one more case.

@tomprats i tried your meta fix but that only stops the automatic translate, if a user actively says translate this page this wont stop it.

For the record here is a image of how the translate feature pollutes the parent element with alot of font HTML Tags

skaermbillede 2018-06-01 kl 11 35 23
shofer commented 6 years ago

@developit we're running in the same issue, can we somehow workaround it until there is a proper solution?

developit commented 6 years ago

I would suggest maybe turning off auto translation. Not really sure of a fix, this is kinda a Chrome bug

Nopzen commented 6 years ago

@developit i happens aswell if the user actively translate the page.

I know we cant guard this, but it would be Nice if the dom dont bloat up as it does. All i’ve Been able to debug on this issue it it seems to happen every time a component updates.

dharFr commented 6 years ago

FYI, in addition to <meta name="google" content="notranslate" /> that disables translation for the whole page, it is also possible to disable translation for a specific section of the page only, using class="notranslate" on the root element of the area that should not be translated (see https://cloud.google.com/translate/faq#technical_questions).

Kanaye commented 6 years ago

Hmm I don't think it 's related to storing state within the DOM. I've experimented with storing preacts state within a WeakMap<Element, State> (just for testing). The example from above shows the same behavior: https://j4y0pj9nnv.codesandbox.io/ source

Kanaye commented 6 years ago

I've debugged this some more. The issue is the combination of preacts diff algorithm and chrome inserting font elements instead of "just" replacing the text. The issue on preacts side is caused by this line: https://github.com/developit/preact/blob/86361f020ffb85cea9aac6af5a07f2682da425ed/src/vdom/diff.js#L206

The font element a inserted by chrome has no props (how should it have any ;) ), the font tag also has no splitText method. Because we are "rerendering" the component isHydrating is false. So the node doesn't get added to innerDiffNodes children array and therefore can't be removed when the diff is done. When preact tries to diff the text vnode it can't find a textNode in the children array (in fact the children array is completely empty) so preact creates a new textNode and inserts it at the first position.

Chrome then again replaces the new textNode as part of the translation process with a font element and the cycle repeats with preacts next diff.

kevinfarrugia commented 6 years ago

Had to disable Chrome's translations by adding translate="no" and class="notranslate" to the <html> element, but definitely not a suitable workaround.

gijo-varghese commented 5 years ago

@Kanaye is there a solution for this?

JimLiu commented 5 years ago

A temporary solution is we can disable re-render for those text elements:

  1. create a NoReRender component like this:
    
    import { h, Component } from 'preact';

export default class NoReRender extends Component { shouldComponentUpdate() { return false; }

render() { const { tagName, children, ...props } = this.props; const TagName = tagName || 'span'; return <TagName {...props}>{children}; } }


2. Update those elements which might be affected with `NoReRender`

e.g.
change
```jsx
      <a
        href={href}
        className="my-link"
      >
        My Link
      </a>

to

      <NoReRender
        tagName="a"
        href={href}
        className="my-link"
        key={'set an unique key is very import'}
      >
        My Link
      </NoReRender>
developit commented 4 years ago

Hi all,

I finally got some time to write up a resilient solution to this: https://gist.github.com/developit/3e807962630ddbd4977cbd07b597f24f

It intercepts Google Translate when it goes to replace Text nodes with its elements, turns the into a mock-text-node (from Preact's perspective), and rewrites some properties that cause it to be diffed as if it were still the same Text node. It also intercepts updates to the fake "text" node and triggers replacement in order to ensure Google Translate updates the new text with a translated version inline.

JoviDeCroock commented 4 years ago

Since there's a workaround, let's close this up, if there are any more issues feel free to make a new issue/reopen!

jacobrask commented 3 years ago

Even after including this patch we're seeing duplicate nodes (original language and translation). We have some parts of the page in Preact and some in React while migrating, and the React parts are working as expected.

Is there a way to debug the patch somehow? I'm not seeing the _font or _data properties on the font nodes, but Element.prototype.insertBefore shows

ƒ (n,r){if(r&&3===r.nodeType&&n&&"font"===n.localName)for(var i in n)if(i.startsWith("_gt_")){r._font=n,n._data=r.data,Object.defineProperties(n,e);break}return t.apply(this,arguments)}

in the console so the patch seems to be applied correctly.

jacobrask commented 3 years ago

In fact it seems like insertBefore is not triggered at all by Google Translate, so maybe Google Translate has changed since the patch was written.

mattias-lonn commented 3 years ago

Same here, I can't get this to work. I am trying to import the patch to my /src/index.js file but the result is the same.

nwesthoff commented 3 years ago

We're experiencing issues as well. Our stack consists of a few things that might also cause this issue:

  • Preact 10.5.13
  • Next.js (with SSG) 10.1.3
  • react-query 2.26.4
  • react-intl 5.15.8

This may or may not have anything to do with the problem, but curious to hear if @jacobrask / @mattias-lonn are using a similar stack. As it might be a combination of these things. I haven't been able to create a small reproduction (yet). React-query is mostly interesting since it's firing immediately after re-focussing on the window (eg. closing the translate window). So everything is re-rendered and showing duplicated strings.

Funnily enough, the original demo isn't an issue for me anymore. So it might be this is caused by something else?

jacobrask commented 3 years ago

Ours is a small app without Next.js or your other dependencies. After debugging this further and setting breakpoints in the translate code in Chrome, I could see that in the context where insertBefore was executed, the patch is not applied. Not sure if this is a timing issue or something else.

If someone wants to troubleshoot further - open the Network tab and find the request being made to Google Translate after translating a page. Find the Initiator script, and there you can search for insertBefore and set breakpoints in the Chrome translate script.

mattias-lonn commented 3 years ago

@nwesthoff The original demo has a state error, that's probably why it doesn't work. My stack is a bit different from yours, the only dependency we have in common is preact. My current stack:

  • preact: 10.5.13
  • preact-cli: 3.0.5
  • preact-render-to-string: 2.1.12
  • preact-router: 3.2.1
lf94 commented 3 years ago

For those hitting the same issue, here is a solution which I have just tested: https://gist.github.com/developit/3e807962630ddbd4977cbd07b597f24f#gistcomment-3729943

Unfortunately I think Google Translate should expose some sort of flag to know the incoming insertBefore call is from it.

oshell commented 2 years ago

I updated the gist from If94 to work with latest version of preact and google translate. https://gist.github.com/developit/3e807962630ddbd4977cbd07b597f24f#gistcomment-3966828

tbgse commented 2 years ago

@JoviDeCroock would it be possible to reopen this issue? It actually crashes the entire browser when testing in Edge and Chrome still duplicates entries etc.

The original proposed resilent solution does not work anymore for us and others, and all the other solutions proposed so far also have certain caveats (e.g. flash of content etc.) - I think it'd be really nice if this could be solved on the framework level instead of having a separate fix, which is basically required because of how popular the google translate feature is.

orosro commented 2 years ago

I can confirm that the original proposal doesn't work anymore, at least in combination with plugins such as: https://gtranslate.io/ which I presume are using google translate under the hood.

ugnius-s commented 1 year ago

How is this not a priority?