Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Firefox 55: no img tag render #194

Open
Kuranes opened this issue Aug 8, 2017 · 14 comments
Open

Firefox 55: no img tag render #194

Kuranes opened this issue Aug 8, 2017 · 14 comments

Comments

@Kuranes
Copy link
Contributor

Kuranes commented Aug 8, 2017

  • Firefox 55 of today, doesn't render the "img" tag. (but does render the "background css image")

Here's a simple clear repro https://jsfiddle.net/pskfb/dhvLvb6q/

  • As an aside, ms Edge browser does the contrary (render the "img" but not the "background css" in the Canvas (but ok in the Result Image)).
@cburgmer
Copy link
Owner

cburgmer commented Aug 9, 2017

Thanks, good find. This might be issue https://bugzilla.mozilla.org/show_bug.cgi?id=789249 resurfacing in different form (not background this time). However that should be checked on the bug tracker. Would you be open to opening an issue there?

@Kuranes
Copy link
Contributor Author

Kuranes commented Aug 10, 2017

Even with preload it doesn't work here.
It does draw in a foreign object tag too.

  • I udpated the sample with image on load and even a regular draw every 1.5 sec and it never draws
  • updated the repro with foreign object along the rest.

https://jsfiddle.net/pskfb/dhvLvb6q/

perhaps I have to find how to extract the foreign object from inside the libraray before the canvas draw, so that we can pinpoint to firefox the bug.

@AustinBrock
Copy link

Having the same issue here. Is it still a Firefox issue?

@cburgmer
Copy link
Owner

I haven't put any time into that, but it seems that it started breaking with the new Firefox version.

The test under ./test/manualIntegrationTestForWebkit.html has worked up to the previous version, now the green image is not rendered.

Unless @Kuranes has filed a report with Firefox, it might be a good option to try this now. Happy for any help trying to setup a minimal test case (not using this library) to demo this on the bug tracker and raising this with the Firefox people.

@Kuranes
Copy link
Contributor Author

Kuranes commented Sep 28, 2017

For now have two samples showing bug cases on Edge and Firefox, but not clear enough imho for bug report

I try to go for full explanation and repro steps, but didn't finished.

Caught full time and forgot to follow up on that, but would love code review or fiddle fork with better/clearer repro ?

@insekkei
Copy link

After I change the rasterizeHTML version from 1.2.4 to 1.2.2, firefox 55 rendered <img ... /> corectly

@cburgmer
Copy link
Owner

Thanks @insekkei, but I can't reproduce it looking at ./test/manualIntegrationTestForWebkit.html when I checkout tag 1.2.2 locally (while Chrome still looks good).
To reproduce from source you need to run npm test once before opening the page. For Chrome you need to additionally serve it through a local webserver, sorry about that.

@cburgmer
Copy link
Owner

@Kuranes Sorry for the long break. I've fiddled with your example, and managed to come up with something close to minimal, that also fixed an issue I had with the image not being properly loaded (unsure whether it's Firefox or jsfiddle eating the self closing "/"): https://jsfiddle.net/7v2ue7r2/12/
This example now renders in Chrome and Safari (reload necessary there) but not fully in Firefox, the background image is there, the regular image missing.

I'd be grateful if you could raise this with Firefox.

@cburgmer
Copy link
Owner

It looks fine for me on 58 & 59. Can somebody confirm?

@skhameneh
Copy link

skhameneh commented Nov 13, 2018

I still have some issues (potentially related) with Firefox, mainly when background-clip: text is used, text does not always render.
1.2.2 is more stable (and will render on the majority of passes), 1.3.0 is very unstable and rarely renders text.

Not: I am not using external fonts, this is with system fonts.

@cburgmer
Copy link
Owner

I would be interesting to know why 1.3.0 looks less reliable. One hunch, relying on native promises in the latest versions (contrary to the homegrown promises before) might have changed the timing aspect.

Even though this library will inline all external references, Firefox especially still has an asynchronous code path involved when rendering inlined items. Adding an additional wait can make this a bit better, but don't guarantee a perfect solution. HTML doesn't offer events to listen for all those resources being rendered.

@skhameneh
Copy link

skhameneh commented Nov 13, 2018

Hey @cburgmer, I have a wrapping solution to adjust a number of issues (including this).
I'll be publishing it soon as a separate module, with rasterizeHTML as a dependency.

Some personal notes:

  • context.imageSmoothingEnabled = false helps reduce unintended anti-aliasing on image copies
  • Waiting on requestAnimationFrame does not resolve background-clip: text nuances in Firefox
  • Using context.scale(devicePixelRatio, devicePixelRatio) can help solve some blurring on HDPI devices, etc.
  • Using canvas.getContext('2d', { alpha: false }) enables sub-pixel anti-aliasing of fonts (reproduces browser functionality better).
  • Waiting 10ms seems to resolve my issues, but I imagine this could be subject to race conditions (Firefox only, Chrome works fine).

I am using width/height on the canvas with a devicePixelRatio multiplier in conjunction with context.scale(devicePixelRatio, devicePixelRatio) and setting the style.height/style.width to the desired size. This has given me a crisp rendering of fonts and images on some devices. Currently I blindly set the pixel ratio to 2, I haven't looked into it further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants
@cburgmer @Kuranes @skhameneh @AustinBrock @insekkei and others