-
Notifications
You must be signed in to change notification settings - Fork 389
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix DOCX font size converting issue (#1905) #1906
Fix DOCX font size converting issue (#1905) #1906
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are many magic numbers. Moreover the range of all size values is not covered, except I suppose to default to 10pt. What if size is 8, -3, +5, or 100? Is 10pt fine of all those? Can the sizes specified in the DOM have units, like pt or px or are they purely numbers? What's special about size.length > 2 such that you know to ignore all but the first two values? I guess you assume then to consider only with +/- prefix and only up to the value 9.
It's not possible for fontSize to be null or have length not > 0 as tested later one.
The W3C documentation describe the allowed values: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/font Therefore values different from this range are not allowed and the default "10pt" is based on the BIRT default which is "10pt". Further answers to your questions:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The explain makes it all clear now. Thanks! 👍
Will this be the final change for 4.17 such that I can do another milestone build after this is merged? |
Yes, this is the last change of 4.17 from my side :o) |
No description provided.