Closed Bug 1583736 Opened 5 months ago Closed 5 months ago

CSS radial-gradient should not accept negative radii

Categories

(Core :: CSS Parsing and Computation, defect, P3)

71 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox71 --- fixed

People

(Reporter: jieorlin, Assigned: emilio)

Details

(Keywords: dev-doc-complete, site-compat)

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/77.0.3865.90 Safari/537.36

Steps to reproduce:

https://drafts.csswg.org/css-images-3/#radial-gradients
<length>
Negative values are invalid.
<length-percentage>{2}
Negative values are invalid.

background-image: radial-gradient(-100px 20px, red, green);
background-image: radial-gradient(100px -20px, red, green);

Chrome fixed in https://bugs.chromium.org/p/chromium/issues/detail?id=978790

Component: Untriaged → Layout
Product: Firefox → Core

This should be relatively easy to fix, and background-image-invalid.html tests this already and should start passing.

Status: UNCONFIRMED → NEW
Component: Layout → CSS Parsing and Computation
Ever confirmed: true
Priority: -- → P3

I wrote a patch, but in the process realized that Blink still supports negative radii in WebKit gradients...

I think we should update either both or none.

Mostly renaming for clarity, as the gradient parsing code is a bit hairy.

This also changes -webkit- gradients, which is, I think, the right thing to do
(otherwise I need to give up on the type system and sprinkle parse_non_negatives
around, which would be unfortunate).

I filed https://bugs.chromium.org/p/chromium/issues/detail?id=1008112 on
Chromium still accepting negative radii for those, so will wait to submit the
patch for review until they reply there with their intentions.

I'm not sure if we should change the behavior of radial-gradient with prefixes, because in theory these are outdated, and we can keep the status quo without updating it. Is it better to go to the CSSWG to discuss?

I can update the Autoprefixer to ensure that the correct radial-gradient with the prefix are output.

Chrome has fixed this issue in #1008112, so should -moz-radial-gradient() also fix negative values?

(In reply to jieorlin from comment #5)

Chrome has fixed this issue in #1008112, so should -moz-radial-gradient() also fix negative values?

Yes. All of them.

Assignee: nobody → emilio
Keywords: site-compat
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b6d1f8ea5b48
Don't allow negative radii in radial gradients. r=boris
Pushed by cbrindusan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/949bff3964d9
Fix Eslint failure due to Delete `,` r=me CLOSED TREE

We probably need to update formal syntax, etc., in the docs, to cover this. And add a rel note to the relevant version.

Keywords: dev-doc-needed
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
You need to log in before you can comment on or make changes to this bug.