Summary: | Fix unitless usage of mathsize | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Rob Buis <rbuis> | ||||||||
Component: | MathML | Assignee: | 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
Rob Buis
2019-02-22 01:38:22 PST
Created attachment 362708 [details]
Patch
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.
Comment on attachment 362708 [details] Patch Clearing flags on attachment: 362708 Committed r241942: <https://trac.webkit.org/changeset/241942> All reviewed patches have been landed. Closing bug. Oh, no! Don’t add more uses of String::format! (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 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. (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. Reopening to attach new patch. Created attachment 362888 [details]
Patch
Already fixed. Reopening to attach new patch. Created attachment 362890 [details]
Patch
Well, I re-landed my patch that fixes it. You are welcome to refine further. 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. |