1451684 - Shape Path Editor seems to default to farthest-side for circle

Status: VERIFIED FIXED

(DevTools :: Inspector, defect, P3)

Description

[Martin Balfanz [:mbalfanz]]

Attachment: shape-editor-radius.jpg

The spec [1] defines that the default radius on circles is "closest-side" when nothing is provided.

><shape-radius>
>= <length> | <percentage> | closest-side | farthest-side
>    Defines a radius for a circle or ellipse. If omitted it defaults to closest-side.

However, the shape path editor seems to default to farthest-side by default and ignore situations with no value or closest-side specified

Please see the attached screenshot. All editor "outlines" appear the same, even though the the radius is different. Please also note that on the first and second screen the content is inside the circle boundaries, showing that the platform indeed defaults to closest-side but the highlighter is off.

STR:
1. Go to about:config and set "layout.css.shape-outside.enabled" to true.
2. Go to http://labs.jensimmons.com/2016/examples/shapes-1.html
3. Inspect the image and toggle on the shape path editor

Expected result:
- The highlighter shows a default radius of closest-side

Actual result:
- The highlighter shows a default radius of farthest-side



[1]: https://www.w3.org/TR/css-shapes-1/#typedef-shape-radius

Comment 1

[Razvan Caliman [:rcaliman]]

Attachment: Bug 1451684 - Compute farthest-side / closest-side circle radius as pixels, then resolve to percentages according to formula defined in spec.

Review commit: https://reviewboard.mozilla.org/r/252336/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/252336/

Comment 2

[Gabriel Luong [:gl] (ΦωΦ)]

Comment on attachment 8987104 [details]
Bug 1451684 - Compute farthest-side / closest-side circle radius as pixels, then resolve to percentages according to formula defined in spec.

https://reviewboard.mozilla.org/r/252336/#review258788

::: devtools/server/actors/highlighters/shapes.js:1886
(Diff revision 1)
>  
>      // Percentage values for circle() are resolved from the
>      // used width and height of the reference box as sqrt(width^2+height^2)/sqrt(2).
>      const computedSize = Math.sqrt((width ** 2) + (height ** 2)) / Math.sqrt(2);
>  
> +    // Position of coordinatees for circle center in pixels.

s/coordinatees/coordinates

"Position coordinates" might sound better here.

Comment 3

[Razvan Caliman [:rcaliman]]

The bug stems from mistakenly computing percentage values (closest-side/farthest-side is converted internally to percentages). 

For `circle()`, a radius expressed in percentages must be computed according to the formula described in the CSS Shapes Level 1 spec defined here: https://www.w3.org/TR/css-shapes-1/#funcdef-circle

Comment 4

[Razvan Caliman [:rcaliman]]

Comment on attachment 8987104 [details]
Bug 1451684 - Compute farthest-side / closest-side circle radius as pixels, then resolve to percentages according to formula defined in spec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/252336/diff/1-2/

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/33bc3b313306
Compute farthest-side / closest-side circle radius as pixels, then resolve to percentages according to formula defined in spec. r=gl

Comment 6

[Razvan Caliman [:rcaliman]]

Try https://treeherder.mozilla.org/#/jobs?repo=try&revision=ad186558d00e18a57f3a2817577f0bb188f0948e&selectedJob=184390438

Comment 7

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/33bc3b313306

Comment 8

[Rares Doghi, Desktop QA]

Hi, I retested this issue in version 62.0a1 (2018-06-24) , and the issue no longer occurs. i will mark it Verified as fixed.