Computed value of border-radius with calc(percentage) is incorrect

VERIFIED FIXED in Firefox 67

Status

()

defect
P2
normal
VERIFIED FIXED
5 months ago
3 months ago

People

(Reporter: bugzilla, Assigned: emilio)

Tracking

(Blocks 1 bug, {testcase})

Trunk
mozilla67
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox66 wontfix, firefox67 fixed)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Reporter

Description

5 months ago
Reduced test
- - - - - - 

http://www.gtalbot.org/BrowserBugsSection/CSS3Values/Bugzilla-getComputedStyle-border-radius.html



Actual result: 8 sub-test failures
eg: the reported computed value of 'border-top-left-radius: calc(50%)' is '0px + 50%'


Expected result: 8 sub-test successes
eg: the reported computed value of 'border-top-left-radius: calc(50%)' should be '50%'


Notes
-----
- The original test is:
https://chromium.googlesource.com/chromium/src/+/c825d655f6aaf73484f9d56e9012793f5b9668cc/third_party/WebKit/LayoutTests/css3/calc/getComputedStyle-border-radius.html
- I get actual results with Firefox 60.4.0 ESR and with Firefox 66.0a1 buildID=20181226093642
- Chromium 71.0.3578.80 and Epiphany 3.22.7 (WebKit 2.18.6) pass this test
- I searched for a duplicate and did not find any.
Reporter

Updated

5 months ago
Blocks: 1376206
Keywords: testcase
Assignee

Comment 1

5 months ago
Always a good excuse to do some cleanup.

Do you plan to submit that test to WPT?
Assignee: nobody → emilio
Flags: needinfo?(bugzilla)
Assignee

Comment 2

5 months ago
I'll add a test if Gerald is not planning to submit it to WPT soon-ish.
Reporter

Comment 3

5 months ago
Emilio,

Yes, my initial plan was to submit the original  
(...) third_party/WebKit/LayoutTests/css3/calc/getComputedStyle-border-radius.html
test to WPT. 

I may also submit that Bugzilla-getComputedStyle-border-radius.html test to WPT; I did not consider doing so. I was mainly thinking, focusing about filing a bug report when I created such test. But now that you have brought up this question, it makes sense to submit it too.
Flags: needinfo?(bugzilla)
Priority: -- → P2
Assignee

Updated

5 months ago
Depends on: 1517511
Reporter

Comment 4

4 months ago

Emilio,

I hope to submit that Bugzilla-getComputedStyle-border-radius.html test under a different and more suitable filename to WPT this week.

As for
.../third_party/WebKit/LayoutTests/css3/calc/getComputedStyle-border-radius.html
it is not clear right now if and if so, when and how it will be submitted to WPT. But the initial desire is to submit it too.

Assignee

Comment 5

4 months ago

Yup, thanks Gérard! I got somewhat driven away from this trying to clean up all the things, because the patch as-is was getting a few failures. Getting almost there :)

Reporter

Comment 6

4 months ago

The filename is

getComputedStyle-border-radius-002.html

and I have created a pull request to submit it to the web platform tests repository:

https://github.com/web-platform-tests/wpt/pull/14987

Integration tests have passed, so now, it is only a matter of getting a reviewer to merge the pull request.

Attachment #9033365 - Attachment is obsolete: true
Assignee

Comment 9

3 months ago

The euclid size is not really used for anything. Also rename it to Size2D to
avoid cbindgen conflicts with values::length::Size.

Depends on D20958

Assignee

Comment 10

3 months ago

The test in https://github.com/web-platform-tests/wpt/pull/15423 hasn't been
synced over yet, but it passes with this patch of course.

Depends on D20959

Comment 11

3 months ago
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/96e8b4cb4561
Simplify border-radius serialization. r=firefox-style-system-reviewers,boris
https://hg.mozilla.org/integration/autoland/rev/ec3876470b16
Make the generic size not use euclid under the hood. r=firefox-style-system-reviewers,boris

Comment 12

3 months ago
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/43862dc87e0b
Use rust lengths for border corners. r=boris

Comment 13

3 months ago
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/a460a4d0a66c
Add some braces on a CLOSED TREE. r=me

Comment 14

3 months ago
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/180c7672c57c
Remove an extra brace on a CLOSED TREE, whoops. r=me
Assignee

Comment 16

3 months ago

Oh, it was a bit tricky to find the actual error from the log:

/builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/ServoStyleConsts.h:1456:12: runtime error: load of value 228, which is not a valid value for type 'bool'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/ServoStyleConsts.h:1456:12 in
Assignee

Comment 17

3 months ago

Well that's interesting. I guess I can do an ASAN build.

Comment 18

3 months ago
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/85f7a0580ce9
Use rust lengths for border corners. r=boris
Assignee

Comment 19

3 months ago

Oh blerg, of course, StyleBasicShape::mRadius gets compared unconditionally, but only initialized in the Inset case. I didn't realize that nsStyleCorners was PodZero'd

Flags: needinfo?(emilio)

Comment 20

3 months ago
bugherder
Status: NEW → RESOLVED
Last Resolved: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Reporter

Comment 21

3 months ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.