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

VERIFIED FIXED in Firefox 67

Status

()

defect
P2
normal
VERIFIED FIXED
7 months ago
5 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)

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.
Blocks: 1376206
Keywords: testcase
Always a good excuse to do some cleanup.

Do you plan to submit that test to WPT?
Assignee: nobody → emilio
Flags: needinfo?(bugzilla)
I'll add a test if Gerald is not planning to submit it to WPT soon-ish.
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
Depends on: 1517511

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.

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 :)

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

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

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

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
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/43862dc87e0b
Use rust lengths for border corners. r=boris
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/a460a4d0a66c
Add some braces on a CLOSED TREE. r=me
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/180c7672c57c
Remove an extra brace on a CLOSED TREE, whoops. r=me

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

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

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

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)
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.