1780240 - html-to-image css grid styling not working properly on latest Firefox

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[hristo_iankov]

Attachment: bug-report.png

Steps to reproduce:

Example URL:
https://jsfiddle.net/51ax8p0j/

Steps to reproduce:

1. Install Firefox (Version 102.0.1)
2. Create an element with display:grid, grid: 33% 33% 33% / 1fr 1fr, and grid-auto-flow: column.
3. Use any html to canvas library (Ex. html-to-image, html-2-canvas, dom-to-image)
4. Render the new canvas on the page and compare to the original html element.

Actual results:

The html and canvas do not look the same. It appears that the grid-auto-flow: column gets removed and defaults to row.

Expected results:

The html and canvas should look the same.

Comment 1

[hristo_iankov]

The linked jsfiddle can be tested on different browser to show it is working (Chrome 103 & Edge 103) and possibly on older versions of Firefox (untested).

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: (html-to-image.js resource used by the testcase)

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: (html2canvas.min.js resource used by the testcase)

Comment 4

[Daniel Holbert [:dholbert]]

I've attached both JS files that the JSFiddle testcase uses, which I downloaded from here (where the testcase was referencing them):
https://cdnjs.cloudflare.com/ajax/libs/html2canvas/1.4.1/html2canvas.min.js
https://cdnjs.cloudflare.com/ajax/libs/html-to-image/1.9.0/html-to-image.js

Testcase itself coming up next.

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: testcase 1 (from reporter's jsfiddle)

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: testcase 2 (reduced somewhat)

Comment 7

[Daniel Holbert [:dholbert]]

At first glance, it looks like the grid-auto-flow: column styling is just getting ignored when html2canvas.js does its thing. (only in Firefox)

The JS library is extremely-minified, which makes it somewhat hard to follow what precisely it's doing... But it looks like there's an un-minified version at https://html2canvas.hertzen.com/dist/html2canvas.js

Comment 8

[Daniel Holbert [:dholbert]]

Attachment: (un-minified html2canvas.js resource used by later testcases)

Comment 9

[Daniel Holbert [:dholbert]]

Attachment: testcase 3 (same as 2 but now with the un-minified html2canvas library)

Comment 10

[Daniel Holbert [:dholbert]]

Oops, it looks like html2canvas.js isn't actually used at all in the original jsfiddle testcase. (It's referenced but unused.)

html-to-image.js is the only script that's actually used there, and googling isn't immediately turning up un-minified source for that one...

Comment 11

[Emilio Cobos Álvarez (:emilio)]

One thing I noticed is that getComputedStyle(document.querySelector(".content")).grid seems wrong (doesn't seem to produce / column, plus it's unresolved). I haven't checked whether that fixes it, but I can tomorrow.

Comment 12

[Daniel Holbert [:dholbert]]

OK, so the html-to-image.js library enumerates the stylesheets' cssText and copies that into a stylesheet inside of a SVG document (which it then draws to a canvas).

And in this case, we seem to be improperly serializing cssText (in part because of the combination of the grid shorthand-property and the grid-auto-flow longhand property that follows it while also being one of the things that the shorthand would set). We're excluding the longhand property from our serialized cssText.

I'll post a more-targeted testcase that demonstrates that.

Comment 13

[Emilio Cobos Álvarez (:emilio)]

Hah, so comment 11 was close enough, and presumably it will fix it :P

Will take a look tomorrow if nobody gets to it sooner.

Comment 14

[Daniel Holbert [:dholbert]]

Attachment: testcase 4 (just showing the cssText)

Comment 15

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1780240 - Properly fail to serialize grid shorthand when not roundtripping. r=dholbert

Other browsers also don't roundtrip properly, but they fail less
severely.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fd5fa5692921
Properly fail to serialize grid shorthand when not roundtripping. r=dholbert

Comment 17

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/35239 for changes under testing/web-platform/tests

Comment 18

[Cristian Tuns]

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

Comment 19

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 20

[Daniel Holbert [:dholbert]]

Verified fixed -- fiddle looks correct in Nightly 105.0a1 (2022-07-27) -- the numbers flow in columns in both parts of the testcase (rather than just in the left part).

Thanks again for the bug report!

Comment 21

[Daniel Holbert [:dholbert]]

Also, for the record, this bug goes back quite a ways -- e.g. Firefox Nightly 66 2019-01-24 matches our recent before-the-fix-landed Nightly rendering of the original testcase and the reduced testcase 4 (and presumably the other testcases which are just minor-variants).

So: not a recent regression. Let's just let this fix ride the trains to release in Firefox 105, I think (no need to think about uplifts/etc).

Comment 22

[hristo_iankov]

Thank you guys for the quick response!

Comment 23

[Alin Ilea, Desktop QA]

Reproduced in Release 104.0
Verified - Fixed in Beta 105.0b4 and the latest Nightly 106.0a1 (2022-08-30) using Windows 10, Ubuntu 20 and macOS 12.