Bug 194940

Summary: Fix unitless usage of mathsize
Product: WebKit Reporter: Rob Buis <rbuis>
Component: MathMLAssignee: Rob Buis <rbuis>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, dbarton, ews-watchlist, fred.wang, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Rob Buis
Reported 2019-02-22 01:38:22 PST
Computation of unitless values for mathsize is wrong, causing these tests to fail: * http://www.w3c-test.org/mathml/relations/css-styling/lengths-1.html * http://www.w3c-test.org/mathml/relations/css-styling/lengths-3.html
Attachments
Patch (3.97 KB, patch)
2019-02-22 01:40 PST, Rob Buis 		no flags
DetailsFormatted DiffDiff
Patch (1.58 KB, patch)
2019-02-25 00:32 PST, Rob Buis 		no flags
DetailsFormatted DiffDiff
Patch (1.77 KB, patch)
2019-02-25 02:06 PST, Rob Buis 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Rob Buis
Comment 1 2019-02-22 01:40:31 PST
Created attachment 362708 [details] Patch
Frédéric Wang (:fredw)
Comment 2 2019-02-22 01:51:11 PST
Comment on attachment 362708 [details] Patch Great thanks. I guess it's a bit hacky and I think Gecko has similar hacks. Probably nonzero unitless values should not be supported.
WebKit Commit Bot
Comment 3 2019-02-22 02:42:49 PST
Comment on attachment 362708 [details] Patch Clearing flags on attachment: 362708 Committed r241942: <https://trac.webkit.org/changeset/241942>
WebKit Commit Bot
Comment 4 2019-02-22 02:42:50 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 5 2019-02-22 02:43:22 PST
<rdar://problem/48306931>
Darin Adler
Comment 6 2019-02-22 09:58:06 PST
Oh, no! Don’t add more uses of String::format!
Frédéric Wang (:fredw)
Comment 7 2019-02-22 10:06:43 PST
(In reply to Frédéric Wang (:fredw) from comment #2) > Comment on attachment 362708 [details] > Patch > > Great thanks. I guess it's a bit hacky and I think Gecko has similar hacks. > Probably nonzero unitless values should not be supported. Let's ask the MathML CG to remove these special parsing so that we don't need these hacks anymore: https://github.com/mathml-refresh/mathml/issues/24 https://github.com/mathml-refresh/mathml/issues/7
Frédéric Wang (:fredw)
Comment 8 2019-02-22 10:54:44 PST
Comment on attachment 362708 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=362708&action=review > Source/WebCore/mathml/MathMLElement.cpp:105 > + return String::format("%.3f%%", unitlessValue * 100.0); I suspect this is not super reliable. I seems to get rounding errors with my local build.
Rob Buis
Comment 9 2019-02-22 11:35:08 PST
(In reply to Darin Adler from comment #6) > Oh, no! Don’t add more uses of String::format! Oops, I was not aware of https://bugs.webkit.org/show_bug.cgi?id=194752! Maybe it could be renamed to deprecatedFormat. I am happy to fix it in a follow up though.
Rob Buis
Comment 10 2019-02-25 00:32:14 PST
Reopening to attach new patch.
Rob Buis
Comment 11 2019-02-25 00:32:16 PST
Created attachment 362888 [details] Patch
Rob Buis
Comment 12 2019-02-25 00:48:34 PST
Already fixed.
Rob Buis
Comment 13 2019-02-25 02:06:29 PST
Reopening to attach new patch.
Rob Buis
Comment 14 2019-02-25 02:06:31 PST
Created attachment 362890 [details] Patch
Darin Adler
Comment 15 2019-03-03 00:32:10 PST
Well, I re-landed my patch that fixes it. You are welcome to refine further.
Rob Buis
Comment 16 2019-03-04 01:21:58 PST
Hi Darin, (In reply to Darin Adler from comment #15) > Well, I re-landed my patch that fixes it. You are welcome to refine further. Thanks for the heads up. We are happy with the change so I will close this bug. Note that we are still evaluating usage of unitless values in mathsize for the next version of the MathML spec, but if we drop support we will likely do it in a new bug.
Note You need to log in before you can comment on or make changes to this bug.