The fraction bar thickness should follow the w3c MathML 3 recommendation: http://www.w3.org/TR/MathML3/chapter3.html#presm.mfrac The vertical alignment of the fraction bar should be more accurate, specially with operators (+,=,...).
Created attachment 74749 [details] Patch
Comment on attachment 74749 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=74749&action=review Please respond to or fix these comments before landing. > WebCore/ChangeLog:8 > + Adjustments of the fraction bar default thickness and vertical alignment. > + https://bugs.webkit.org/show_bug.cgi?id=50018 > + > + Test: mathml/presentation/fractions.xhtml It would be nice if you explained what change you actually made. Like why is this new change more correct? > WebCore/mathml/RenderMathMLFraction.cpp:145 > + int adjustForThickness = m_lineThickness > 1 ? static_cast<int>(m_lineThickness / 2) : 1; Why static_cast<int> and not just int(m_lineThickness / 2) ?
(In reply to comment #2) > It would be nice if you explained what change you actually made. Like why is this new change more correct? I changed the default value of the bar thickness from "thin" to "medium" according to the MathML 3 recommendation. I thus had to adjust the numeric value of the "thin", "medium", and "thick" thicknesses for a correct rendering. I also changed the vertical alignment of the bar, to be more reliable with operators like: +,=,... See the removed FIXME in the patch for more details. > Why static_cast<int> and not just int(m_lineThickness / 2) ? There is no good reason. It will be corrected in the revised patch.
Created attachment 77806 [details] Revised patch
Comment on attachment 77806 [details] Revised patch Rejecting attachment 77806 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-4', 'apply-attachment', '--no-update', '--non-interactive', 77806]" exit_code: 2 Last 500 characters of output: to patch. Skipping patch. 1 out of 1 hunk ignored patching file LayoutTests/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file LayoutTests/mathml/presentation/fractions.xhtml patching file LayoutTests/platform/mac/mathml/presentation/fractions-expected.checksum patching file LayoutTests/platform/mac/mathml/presentation/fractions-expected.txt Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Kenneth Rohde Christiansen', u'--force']" exit_code: 1 Full output: http://queues.webkit.org/results/7388126
Created attachment 78401 [details] Regenerated patch The previous patch didn't apply because some MathML related files moved in the tree: WebCore/mathml to WebCore/rendering/mathml.
Comment on attachment 78401 [details] Regenerated patch Clearing flags on attachment: 78401 Committed r75385: <http://trac.webkit.org/changeset/75385>
All reviewed patches have been landed. Closing bug.
This broke Leopard build. I think m_lineThickness should be float instead of double.
(In reply to comment #9) > This broke Leopard build. I think m_lineThickness should be float instead of double. Too bad it didn't ran on the bot. I don't have access to my devel machine right now, could you commit the potential fix? That would be great.
(In reply to comment #10) > (In reply to comment #9) > > This broke Leopard build. I think m_lineThickness should be float instead of double. > > Too bad it didn't ran on the bot. I don't have access to my devel machine right now, could you commit the potential fix? That would be great. I added the cast to float back in the line 160 for now. You can change the type of m_lineThickness to float later if you think my suggestion is right. http://trac.webkit.org/changeset/75394
(In reply to comment #11) > (In reply to comment #10) > > (In reply to comment #9) > > > This broke Leopard build. I think m_lineThickness should be float instead of double. > > > > Too bad it didn't ran on the bot. I don't have access to my devel machine right now, could you commit the potential fix? That would be great. > > I added the cast to float back in the line 160 for now. You can change the type of m_lineThickness to float later if you think my suggestion is right. > > http://trac.webkit.org/changeset/75394 Yes, a float is sufficient here for the accuracy and it is better to avoid an unneeded cast. I'll do the change tomorrow by submitting a new patch.
http://trac.webkit.org/changeset/75385 might have broken GTK Linux 32-bit Release The following tests are not passing: fast/dom/Window/window-property-descriptors.html fast/dom/prototype-inheritance.html fast/js/global-constructors.html
Type change to float made in bug 52201.