1315353 - "border-image: url("data:image/svg+xml") repeat" broken after implementation of space value of border-image-repeat

Status: RESOLVED FIXED

(Core :: Layout, defect, P1)

Description

[mariomario13245]

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0
Build ID: 20161104004016

Steps to reproduce:

I did this:

.viridian-textbox {
	background-color: #1b1b2a;
	color: #a4a4ff;
	border-image: url("data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' width='48' height='48'%3E%3Cg fill='%23a4a4ff' stroke-width='0'%3E%3Cpath d='M2 2h4v44H2z'/%3E%3Cpath d='M2 2h44v4H2z'/%3E%3Cpath d='M42 2h4v44h-4z'/%3E%3Cpath d='M2 42h44v4H2zM8 8h2v32H8z'/%3E%3Cpath d='M8 8h32v2H8z'/%3E%3Cpath d='M38 8h2v32h-2z'/%3E%3Cpath d='M8 38h32v2H8z'/%3E%3C/g%3E%3C/svg%3E") 16 repeat;
}

which _should_ work right.
Normal firefox displays it correctly, but dev edition doesn't.
Here's the site:
http://vvvvvvresource.byethost6.com/


Actual results:

This happened in developer edition:
http://i.imgur.com/9YeIN9X.png


Expected results:

In normal firefox:
http://i.imgur.com/1GqfKiz.png

Comment 1

[Loic]

Attachment: 1315353.html

Comment 2

[Loic]

Regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=2544cbe88839da4d80aee7d1df8cf1f923000a9e&tochange=fa5ba1af68d09baf15b9ac71b05b067997b75477

Ethan Lin — Bug 720531 - Part 1. Implement space of border-image-repeat CSS property. r=dbaron

Ethan, could you check this regression, please.

Comment 3

[Gerry Chang [:gchang]]

Track 51+/52+ as regression related to CSS.

Comment 4

[Ethan Lin[:ethlin]]

(In reply to Loic from comment #2)
> Regression range:
> https://hg.mozilla.org/integration/mozilla-inbound/
> pushloghtml?fromchange=2544cbe88839da4d80aee7d1df8cf1f923000a9e&tochange=fa5b
> a1af68d09baf15b9ac71b05b067997b75477
> 
> Ethan Lin — Bug 720531 - Part 1. Implement space of border-image-repeat CSS
> property. r=dbaron
> 
> Ethan, could you check this regression, please.

Okay, I'll fix it.

Comment 5

[Ethan Lin[:ethlin]]

Attachment: Part1. Fix image size.

The image size was wrong. I shouldn't do the AppUnits conversion.

Comment 6

[Ethan Lin[:ethlin]]

Attachment: Part2. Add reftests.

I add reftest for border-image-repeat with SVG content.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8808060 [details] [diff] [review]
Part2. Add reftests.

>diff --git a/layout/reftests/w3c-css/submitted/background/border-image-repeat-1-ref.html b/layout/reftests/w3c-css/submitted/background/border-image-repeat-1-ref.html

Maybe call the test border-image-repeat-svg-1.html? (and -ref)  (So that svg is in the name, and it's a little more specific.)

>+    <link rel="help" href="https://www.w3.org/TR/css3-background/#background-repeat">

This should link to border-image-repeat, not background-repeat.

r=dbaron with that

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(I presume you tested that the reftest fails without the patch in addition to testing that it passes with the patch.  If not, please test that.)

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

We looked at this one in platform triage just now. It does not seem like a release blocking issue. If we have a fix ready soon, let's plan to ship in 50.1.0. Please let me know if there are any concerns.

Comment 10

[Ethan Lin[:ethlin]]

(In reply to David Baron :dbaron: ⌚️UTC+8 from comment #8)
> (I presume you tested that the reftest fails without the patch in addition
> to testing that it passes with the patch.  If not, please test that.)

Right, I tested. The reftest failed without the patch.

Comment 11

[Ethan Lin[:ethlin]]

Attachment: Part2. Add reftests. (carry r+: dbaron)

Address dbaron's comments. I changed the reftest filename and corrected the link path.

Comment 12

[Ethan Lin[:ethlin]]

try looks good:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ca3fe52352e5ab92ddd73eb1032f5ba9e06fda92&selectedJob=30802491

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b14c86a0fb84
Part 1: Fix the wrong image size of border-image-repeat while the content is SVG. r=dbaron
https://hg.mozilla.org/integration/mozilla-inbound/rev/6bdf4b5b69d1
Part 2: Add testcase for border-image-repeat with SVG content. r=dbaron

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b14c86a0fb84
https://hg.mozilla.org/mozilla-central/rev/6bdf4b5b69d1

Comment 15

[Ethan Lin[:ethlin]]

Comment on attachment 8808059 [details] [diff] [review]
Part1. Fix image size.

Approval Request Comment
[Feature/regressing bug #]: bug 720531
[User impact if declined]: The display of border-image-repeat may be wrong when the content is SVG. 
[Describe test coverage new/current, TreeHerder]: tested locally and try server
[Risks and why]: Low. The unit of the size was wrong. The patch corrects the unit.
[String/UUID change made/needed]: None.

Comment 16

[Gerry Chang [:gchang]]

Comment on attachment 8808059 [details] [diff] [review]
Part1. Fix image size.

Fix a regression. Aurora51+.

Comment 17

[Iris Hsiao [:ihsiao]]

 https://hg.mozilla.org/releases/mozilla-aurora/rev/1f41acac2aa6

Comment 18

[Gerry Chang [:gchang]]

Comment on attachment 8808059 [details] [diff] [review]
Part1. Fix image size.

Since the merge day is passed, move approval‑mozilla‑beta? flag to approval‑mozilla‑release?

Comment 21

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8808059 [details] [diff] [review]
Part1. Fix image size.

New regression in 50, let's include it in 50.1.0

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/539929d75bed01a844f4f59b0052e2db08af2fa8

Comment 24

[Loic]

Adding keywords as this bug is fixed in the 50.1.0.

Comment 25

[Kohei Yoshino [:kohei]]

Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2016/border-image-is-broken-under-certain-circumstances/