Color-stop matching fails with TypeError due to faulty regexp

[usmonster]

I'm seeing a TypeError when using color keywords (e.g. transparent, as opposed to rgb() values) since matching against the "step" regex in LinearGradientContainer will always return null in that case (see this line).

Can you please update the regexp to work as expected with both [a-z]+ color keywords as well as the rgb()/rgba() functional notation? Or would you be open to a pull request that does just that? (I can even add hsl()/hsla() support to the regexp, if you want...) Thanks!

[niklasvh]

A pull request with the fix and tests to support it would be excellent!

[usmonster]

I'll try to take a look this weekend. Note to self:
https://developer.mozilla.org/docs/Web/CSS/color_value
https://developer.mozilla.org/docs/Web/CSS/gradient
http://stackoverflow.com/a/12456392/357774

[niklasvh]

Regarding implementing border-style inset, there is a need to get a common parsing for colors as well, since it needs to darken/lighten colors by a specific percentage. With this gradient fix, if there is a need for parsing colors into a common format, it could be useful with the border-style: inset as well.

For example, if any color (hex, rgb, rgba, hls, hlsa, predefined name such as 'red', 'green' etc.) could be parsed into Color object with r, g, b and a values defined, it would make it significantly easier to implement the border inset as well.

[usmonster]

Agreed that a common color parsing/representation can and should be reused. I'll keep it in mind and probably have questions.

[usmonster]

Didn't get a chance to look at this over the weekend, will possibly/hopefully have time tomorrow.

[niklasvh]

Any update on this?

[usmonster]

Er, sorry, not yet. :/ Been swamped and had limited computing resources, though I did set up some tests locally (haven't implemented anything yet).

[niklasvh]

I've added basic color parsing (named, rgb, rgba, hex3 and hex6 so far) and changed all uses of color to use those. The gradient regexps haven't been updated yet.

See 313c227

[usmonster]

Nice! I'll take a look this weekend and see about updating those regexps.

[usmonster]

@niklasvh Can you take a look at the commit I just made on my branch (359ee8b) and let me know what you think? I'll remove my superfluous comments and will add tests for color names soon, but can you please also take a look at the other change?

I corrected direction parsing for Firefox, so certain linear gradients that didn't render correctly before should now work. You can see the manual test I just temporarily put in tests/lineargradients_manual.html (basically a copy of tests/cases/background/linear-gradient.html). I suppose I can verify the accuracy increase and no regressions on other browsers/versions once the selenium tests can be run, but can you please check to see that this is okay? I suspect the render accuracy improvement might only be seen comparing more recent versions of FF than those tested.. (Looks like the only version of Firefox tested is 15??)

[usmonster]

Hmm, hold that thought, I may have done something weird, squares on my home machine are mostly black now..

[usmonster]

Ah, nope, looks like 4985279 is the problem commit, my stuff looks ok. 😅

[niklasvh]

What happens if the percentage is something else than 100% or 0%?

[usmonster]

As of right now it doesn't support arbitrary percentages (leaves them as the default 50%), but I can add that easily enough. (Update: added support for arbitrary percentages: 6af1874.)

What I'm not as sure about are non-percentage lengths, which can be in px, em, rem, ex, cm, mm, in, pc, pt, etc... I haven't yet looked for it in the code, but do you already have a way to handle/convert lengths of various units? (If not, should that become part of this feature, or should it be implemented separately?)

[niklasvh]

absolute lengths should be easy to implement, but I think px should be enough to begin with.

[usmonster]

@niklasvh I've just gotten some more time to look at and work on this. Can you take a look at my fix-firefox-gradients branch and comment before I make a pull request?

I should note that, although the regexps can parse pixel lengths (both in gradient start positions and color stop positions), there's still no support for correctly rendering them. I think the cleanest way to add support for absolute lengths would be to include bounds information with the imageData that's passed into the gradient container constructors, and then to convert them to percentages. That should probably go in a separate pull request, though.

[niklasvh]

Thanks! Will have a closer look at this shortly and get back to you with any feedback I might have

[ccemeraldeyes]

Any progress on this? It's still needed!

[usmonster]

I'm ready to make a pull request if @niklasvh can give the approach I used on my branch a thumbs-up..

[ptarjan]

👍 I just ran into this:

TypeError: Cannot read property '1' of null at GradientContainer.<anonymous> (http://localhost:8100/all.js:34496:44) 
at Array.map (native) 
at GradientContainer.LinearGradientContainer (http://localhost:8100/all.js:34493:66)
at ImageLoader.279.ImageLoader.loadImage (http://localhost:8100/all.js:34367:16) 
at ImageLoader.<anonymous> (http://localhost:8100/all.js:34339:46) 
at Array.forEach (native) 
at ImageLoader.279.ImageLoader.addImage (http://localhost:8100/all.js:34337:23) 
at Array.forEach (native) 
at ImageLoader.279.ImageLoader.findBackgroundImage (http://localhost:8100/all.js:34331:71) 
at 295.exports.bind (http://localhost:8100/all.js:36357:25)

[usmonster]

Hey @niklasvh! Thanks for fixing #526.

Did you ever get a chance to look at my fix-firefox-gradients branch? I would rebase my branch (and fix any new conflicts) before I make the PR, but please let me know if a PR would still be welcome. (Do you think you'd be able to review it in a reasonable amount of time?) Thanks!

[niklasvh]

@usmonster No, sorry I missed that. One potential problem I was thinking while doing that fix is that if the browser decides not to convert named colors such as red or blue to their respective rgb or rgba values, simply taking into account transparent wouldn't work. The Color module takes into account all the different color varients, but I am not quite happy with how the current parsing is done in master to begin with.

Any ideas how the named colors could be handled?

[usmonster]

@niklasvh, The lookup and normalization that's already done by the Color constructor should be sufficient to handle all named colors, unless I've misunderstood something?

Additionally, I've fixed up the regular expressions used for parsing colors and linear gradients to make them more robust, correct, and performant, so I don't think named colors should be an issue. What do you think?

[niklasvh]

@usmonster You are right, I was referring to master...usmonster:fix-firefox-gradients#diff-48b5afb6985c457b9f79fcca1cfb499dR21 which I now noticed does take into account named colors, so no problem there. Looks good otherwise, would you mind rebasing and putting up a pull request?

[usmonster]

No problem! I'll try to take a look in the next couple of days--I kind of missed my weekend window. :/

[usmonster]

@niklasvh Please see #708. :)

[Observer999]

I received "TypeError: colorStopMatch is null" at html2canvas.js:1454:13 in FireFox. In Chrome it working.

[usmonster]

Hi @Observer999! This issue is closed. Please search for a similar open issue or create a new issue with a link to a page that reproduces the issue you're having. (Please also specify there which version of Firefox you've tested in.) I can imagine it might have something to do with the TODOs in the code.. Thanks!

[denizgokce]

@usmonster @niklasvh Hello guys I know this issue is closed but I am using the latest version of html2canvas and I am getting the same error in Chrome latest version but in Firefox it is working.

html2canvas.js:formatted:1377 Uncaught (in promise) TypeError: Cannot read property '1' of null
at LinearGradientContainer. (html2canvas.js:formatted:1377)
at Array.map ()
at new LinearGradientContainer (html2canvas.js:formatted:1374)
at ImageLoader.loadImage (html2canvas.js:formatted:1256)
at ImageLoader. (html2canvas.js:formatted:1227)
at Array.forEach ()
at ImageLoader. (html2canvas.js:formatted:1225)
at Array.forEach ()
at ImageLoader.findBackgroundImage (html2canvas.js:formatted:1219)
at html2canvas.js:formatted:2563