1507103 - Shape path editor breaks when attempting to move center point of circle()

Status: VERIFIED FIXED

(DevTools :: Inspector, defect, P1)

Description

[Martin Balfanz [:mbalfanz]]

Attachment: broken-shape-outside-rule

STR:
- visit https://labs.jensimmons.com/2016/examples/shapes-1.html
- open devtools and inspect the .tomato
- enable the shape path editor
- try to move the center point

ER:
- Center point is moved and coordiantes are set in the rules view

AR:
- Editor breaks. Coordinates have a value of "NaNcenter", see screenshot attached

Comment 2

[Razvan Caliman [:rcaliman]]

Attachment: Bug 1507103 - Account for empty circle() and ellipse() in Shape Path Editor. r=gl

This patch handles empty circle() and ellipse() basic shape definitions.

Their computed values will hold default values for center and radius.

For circle(), it is just a case of treating identifiers like "center"
as unitless to allow falling back to computed sizes and pixels.
For ellipse(), the solution is identical, but the issue is compounded
by an incomplete computed style value tracked in Bug 1521508 which is
missing the default values for x and y radii.

Comment 3

[Razvan Caliman [:rcaliman]]

Reduced test cases (run code in the address bar):

For empty circle():

data:text/html,<style>div{width:500px;height:500px;background-color:black; clip-path:circle(); }</style><div>

For empty ellipse():

data:text/html,<style>div{width:500px;height:500px;background-color:black; clip-path:ellipse(); }</style><div>

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e25a1ed32b53
Account for empty circle() and ellipse() in Shape Path Editor. r=gl

Comment 5

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/e25a1ed32b53

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9037963 [details]
Bug 1507103 - Account for empty circle() and ellipse() in Shape Path Editor. r=gl

Too late for 65. This can ride with 66.

Comment 7

[Razvan Caliman [:rcaliman]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #6)

Comment on attachment 9037963 [details]
Bug 1507103 - Account for empty circle() and ellipse() in Shape Path Editor. r=gl

Too late for 65. This can ride with 66.

It's ok. I can understand.

Did the completed approval request form make it to your end on some dashboard? I see the note is missing from the request on Bugzilla and don't know if it was lost in transit or not.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

No, it didn't :(

Comment 9

[Razvan Caliman [:rcaliman]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #8)

No, it didn't :(

Argh! It was a bug, then. I'll make sure to watch out for it in the future. Thanks!

Comment 10

[Razvan Caliman [:rcaliman]]

For context, regression caused by the fix for Bug 1473184.

Comment 11

[Paul_B]

I was able to reproduce the issue on a 66.0b1 build (20190121133415) on Windows 10, Windows 7, Mac OS 10.14.2 and Ubuntu 18.04.

After updating to the latest build - 66.0b3 (20190128143734), the issue is verified as fixed on all OS mentioned above.
The center point can be moved without any issues and the coordinates change accordingly.