Closed Bug 1294171 Opened 3 years ago Closed 3 years ago
Material Design Light SVG checkbox doesn't show check with Firefox 50
58 bytes, text/x-review-board-request
58 bytes, text/x-review-board-request
Google has a CSS/JS package for emulating their Material Design aesthetic on the Web. Their reskin of a standard checkbox is broken in Firefox 50. If you go to https://getmdl.io/components/index.html#lists-section and scroll down to the checkbox demo, you can see the HTML in its native habitat. It should show a blue SVG (in a data URI) checkmark when clicked, but stays empty. Here's a minimal testcase. https://codepen.io/dannyobrien/pen/JKwYap This looks like a regression -- it worked fine in my copy of Firefox 48. I ran mozregression on it, and it ended up at changeset: 373e67c2caaddfec3d3901e038804543c3da9afb pushlog_url: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=373e67c2caaddfec3d3901e038804543c3da9afb&tochange=37c7fec8a0c130033c7e8462b058ea360c6d36d9 ..so it looks like it might be in the SVG rendering code, possibly interacting with it being in a DATA URI?
CJ, might this be from your fix from bug 1275450?
It's a duplication of bug 1293929
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1293929
According to https://github.com/webcompat/web-bugs/issues/2992 this is not fixed, and I think we need to be concerned about shipping this regression.
[Tracking Requested - why for this release]: web compat regression
it's kind of dup of 1287320, but not the totally the same either. This test site give a data url as mask(which is not supported yet before enable-mask-as-shorthand) url("data:image/svg+xml;base64,xxxxx") I think the correct solution for this kind of regression is go back to original behavior(treat unresolvable masks as no-mask) when MASK_AS_SHORTHAND is undefined. Working on the patch
bug 1294171(this one) - This web site five a valid mask value, but not valid for firefox since we are not support image mask yet. bug 1287320 - This web site give a invalid svg mask in a SVG document, which is embedded in a HTML document. (fixed) bug 1293929 - This web site give a invalid svg mask in HTML document. The patch here can accidentally fix bug 1293929 before turning on MOZ_ENABLE_MASK_AS_SHORTHAND.
Comment on attachment 8782252 [details] Bug 1294171 - Part 1. Treat unresolvable mask as no mask before support image mask. https://reviewboard.mozilla.org/r/72444/#review70394
Attachment #8782252 - Flags: review?(mstange) → review+
Comment on attachment 8782273 [details] Bug 1294171 - Part 2. Create a reftest for data url mask. https://reviewboard.mozilla.org/r/72474/#review70396
Attachment #8782273 - Flags: review?(mstange) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/23fcd6d28628 Part 1. Treat unresolvable mask as no mask before support image mask. r=mstange https://hg.mozilla.org/integration/autoland/rev/dab9f89ca163 Part 2. Create a reftest for data url mask. r=mstange
Comment on attachment 8782252 [details] Bug 1294171 - Part 1. Treat unresolvable mask as no mask before support image mask. Approval Request Comment [Feature/regressing bug #]: 1275450 [User impact if declined]: A masked element become invisible if that mask is an image mask. [Describe test coverage new/current, TreeHerder]: manually + try [Risks and why]: low risk. Correct a regression on SVG mask rendering. [String/UUID change made/needed]: Please uplift (part 1) to aurora and beta
This can still make it into the beta 6 build if we can land it by early Monday morning. Andrei can your team help verify the fix? You can check dbaron's link from comment 3 for more cases.
Danny, want to test this from mozilla-central's test builds? >^..^< You can get the appropriate build for your OS from here, you don't even have to wait for tomorrow's Nightly: https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=3da4d64410c002418eab4b025dfb24a6f4f57fd6
Yep, I can confirm that this is now working. Nice job! Thank you so much.
Karl (or Andrei) can you test just to make sure, with Karl's test cases here, https://github.com/webcompat/web-bugs/issues/2992 ?
Comment on attachment 8782252 [details] Bug 1294171 - Part 1. Treat unresolvable mask as no mask before support image mask. Fix for regression in 49, let's uplift this for the beta 6 build on Monday so that we don't ship this to release.
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #27) > Karl (or Andrei) can you test just to make sure, with Karl's test cases > here, https://github.com/webcompat/web-bugs/issues/2992 ? Confirmed. This is fixed! Thanks a lot everyone for the quick fix. With <3 from Web compat.
Since this is already fixed, we don't really need to track it.
I've managed to reproduce the bug using the sample URL from Comment 0, on 50.0a2 (2016-08-10) and 51.0a1 (2016-08-08). I can also confirm that this is verified fixed on Windows 10 x64, Mac OS X 10.11.1 and Ubuntu 16.04 x64, using: - 48.0.2-build1 (20160823121617) - 49.0b6-build1 (20160822111414) - 50.0a2 (2016-08-24) - 51.0a1 (2016-08-24)
You need to log in before you can comment on or make changes to this bug.