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

RESOLVED FIXED in Firefox 50

Status

()

defect
P1
normal
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: mariomario13245, Assigned: ethlin)

Tracking

(4 keywords)

50 Branch
mozilla52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox49 unaffected, firefox50- fixed, firefox51+ fixed, firefox52+ fixed)

Details

Attachments

(3 attachments, 1 obsolete attachment)

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
Posted file 1315353.html
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.
Blocks: 720531
Status: UNCONFIRMED → NEW
Has Regression Range: --- → yes
Has STR: --- → yes
Component: Untriaged → Layout
Ever confirmed: true
Flags: needinfo?(ethlin)
Keywords: regression, testcase
OS: Unspecified → All
Product: Firefox → Core
Hardware: Unspecified → All
Summary: Div border images aren't right in developer edition → "border-image: url("data:image/svg+xml") repeat" broken after implementation of border-image-repeat CSS property
Version: 51 Branch → 50 Branch
Track 51+/52+ as regression related to CSS.
Summary: "border-image: url("data:image/svg+xml") repeat" broken after implementation of border-image-repeat CSS property → "border-image: url("data:image/svg+xml") repeat" broken after implementation of space value of border-image-repeat
(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.
Flags: needinfo?(ethlin)
The image size was wrong. I shouldn't do the AppUnits conversion.
Attachment #8808059 - Flags: review?(dbaron)
Posted patch Part2. Add reftests. (obsolete) — Splinter Review
I add reftest for border-image-repeat with SVG content.
Attachment #8808060 - Flags: review?(dbaron)
Attachment #8808059 - Flags: review?(dbaron) → review+
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
Attachment #8808060 - Flags: review?(dbaron) → review+
(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.)
Assignee: nobody → ethlin
Status: NEW → ASSIGNED
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.
Priority: -- → P1
(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.
Address dbaron's comments. I changed the reftest filename and corrected the link path.
Attachment #8808060 - Attachment is obsolete: true
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
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
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b14c86a0fb84
https://hg.mozilla.org/mozilla-central/rev/6bdf4b5b69d1
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
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.
Attachment #8808059 - Flags: approval-mozilla-beta?
Attachment #8808059 - Flags: approval-mozilla-aurora?
Comment on attachment 8808059 [details] [diff] [review]
Part1. Fix image size.

Fix a regression. Aurora51+.
Attachment #8808059 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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?
Attachment #8808059 - Flags: approval-mozilla-beta? → approval-mozilla-release?
Duplicate of this bug: 1318308
Duplicate of this bug: 1320580
Comment on attachment 8808059 [details] [diff] [review]
Part1. Fix image size.

New regression in 50, let's include it in 50.1.0
Attachment #8808059 - Flags: approval-mozilla-release? → approval-mozilla-release+
Duplicate of this bug: 1321259
Adding keywords as this bug is fixed in the 50.1.0.
You need to log in before you can comment on or make changes to this bug.